Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Server side password selection #658

Merged
merged 20 commits into from
Sep 25, 2020
Merged

Server side password selection #658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e549a0a
Server side password selection
whaught Sep 23, 2020
e35d1f4
include fbinternal
whaught Sep 23, 2020
a321e17
change from fbUser
whaught Sep 24, 2020
f147862
deal with reauth error
whaught Sep 24, 2020
7f19c87
complexity validation
whaught Sep 24, 2020
cccf20c
error string
whaught Sep 24, 2020
f6a1cd4
firebase
whaught Sep 24, 2020
a4a90fb
Merge remote-tracking branch 'upstream/main' into oob-code
whaught Sep 24, 2020
19ae97d
flash
whaught Sep 24, 2020
67b9e42
upstream file
whaught Sep 24, 2020
f53209c
fix flash
whaught Sep 24, 2020
19af617
only mark the server side error
whaught Sep 24, 2020
caf8d29
try to get session anyway
whaught Sep 24, 2020
4b59f83
populate email
whaught Sep 24, 2020
920f777
do everything on the server
whaught Sep 24, 2020
f41c11e
hidden field to get email
whaught Sep 24, 2020
d663ad2
readonly
whaught Sep 24, 2020
1472efc
Update pkg/controller/login/select_password.go
whaught Sep 25, 2020
faf7e80
Update pkg/controller/login/select_password.go
whaught Sep 25, 2020
28cbe5f
review stuff
whaught Sep 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions cmd/server/assets/login/_loginscripts.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,41 @@
</div>
{{end}}

{{define "login/pwd-validate"}}
{{if .requirements.HasRequirements}}
<p class="card-text ml-4">
<small class="form-text text-muted">
<span class="row">Password should be:</span>
{{if gt .requirements.Length 0}}
<span class="row ml-1" id="length-req">
<span id="icon" aria-hidden="true"></span>At least {{.requirements.Length}} characters long
</span>
{{end}}
{{if gt .requirements.Uppercase 0}}
<span class="row ml-1" id="upper-req">
<span id="icon" aria-hidden="true"></span>Contain {{.requirements.Uppercase}} uppercase letter
</span>
{{end}}
{{if gt .requirements.Lowercase 0}}
<span class="row ml-1" id="lower-req">
<span id="icon" aria-hidden="true"></span>Contain {{.requirements.Lowercase}} lowercase letter
</span>
{{end}}
{{if gt .requirements.Number 0}}
<span class="row ml-1" id="num-req">
<span id="icon" aria-hidden="true"></span>Contain {{.requirements.Number}} number
</span>
{{end}}
{{if gt .requirements.Special 0}}
<span class="row ml-1" id="special-req">
<span id="icon" aria-hidden="true"></span>Contain {{.requirements.Special}} special character
</span>
{{end}}
</small>
</p>
{{end}}
{{end}}

{{define "login/pwd-validate-js"}}
{{if .requirements.HasRequirements}}
let $lenReq = $('#length-req');
Expand Down
33 changes: 1 addition & 32 deletions cmd/server/assets/login/change-password.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,38 +39,7 @@
<label for="retype">Retype password</label>
</div>

{{if .requirements.HasRequirements}}
<p class="card-text ml-4">
<small class="form-text text-muted">
<span class="row">Password should be:</span>
{{if gt .requirements.Length 0}}
<span class="row ml-1" id="length-req">
<span id="icon"></span>At least {{.requirements.Length}} characters long
</span>
{{end}}
{{if gt .requirements.Uppercase 0}}
<span class="row ml-1" id="upper-req">
<span id="icon"></span>Contain {{.requirements.Uppercase}} uppercase letter
</span>
{{end}}
{{if gt .requirements.Lowercase 0}}
<span class="row ml-1" id="lower-req">
<span id="icon"></span>Contain {{.requirements.Lowercase}} lowercase letter
</span>
{{end}}
{{if gt .requirements.Number 0}}
<span class="row ml-1" id="num-req">
<span id="icon"></span>Contain {{.requirements.Number}} number
</span>
{{end}}
{{if gt .requirements.Special 0}}
<span class="row ml-1" id="special-req">
<span id="icon"></span>Contain {{.requirements.Special}} special character
</span>
{{end}}
</small>
</p>
{{end}}
{{template "login/pwd-validate" .}}

<button type="submit" id="submit" class="btn btn-primary btn-block" disabled>Set password</button>
</form>
Expand Down
81 changes: 6 additions & 75 deletions cmd/server/assets/login/select-password.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
<head>
{{template "floatingform" .}}
{{template "head" .}}
{{template "firebase" .}}
</head>

<body class="tab-content">
Expand All @@ -18,16 +17,16 @@
<div class="card shadow-sm">
<div class="card-header">Select new password</div>
<div class="card-body">
<form id="loginForm" class="floating-form" action="/login/select-password" method="POST">
<form id="loginForm" class="floating-form" action="/login/select-password?oobCode={{.code}}" method="POST">
{{.csrfField}}
<div class="form-label-group">
<input type="email" id="email" name="email" class="form-control" placeholder="Email address" required
autofocus />
<input type="email" name="email" class="form-control" placeholder="Email address" value="{{.email}}"
required readonly/>
<label for="email">Email address</label>
</div>

<div class="form-label-group mb-2">
<input type="password" id="password" class="form-control" placeholder="Password"
<input type="password" name="password" id="password" class="form-control" placeholder="Password"
autocomplete="new-password" required />
<label for="password">Password</label>
</div>
Expand All @@ -37,38 +36,7 @@
<label for="retype">Retype password</label>
</div>

{{if .requirements.HasRequirements}}
<p class="card-text ml-4">
<small class="form-text text-muted">
<span class="row">Password should be:</span>
{{if gt .requirements.Length 0}}
<span class="row ml-1" id="length-req">
<span id="icon"></span>At least {{.requirements.Length}} characters long
</span>
{{end}}
{{if gt .requirements.Uppercase 0}}
<span class="row ml-1" id="upper-req">
<span id="icon"></span>Contain {{.requirements.Uppercase}} uppercase letter
</span>
{{end}}
{{if gt .requirements.Lowercase 0}}
<span class="row ml-1" id="lower-req">
<span id="icon"></span>Contain {{.requirements.Lowercase}} lowercase letter
</span>
{{end}}
{{if gt .requirements.Number 0}}
<span class="row ml-1" id="num-req">
<span id="icon"></span>Contain {{.requirements.Number}} number
</span>
{{end}}
{{if gt .requirements.Special 0}}
<span class="row ml-1" id="special-req">
<span id="icon"></span>Contain {{.requirements.Special}} special character
</span>
{{end}}
</small>
</p>
{{end}}
{{template "login/pwd-validate" .}}

<button type="submit" id="submit" class="btn btn-primary btn-block">Set password</button>
</form>
Expand All @@ -87,25 +55,9 @@
$(function() {
let $form = $('#loginForm');
let $submit = $('#submit');
let $email = $('#email');
let $password = $('#password');
let $retype = $('#retype');

let urlVars = getUrlVars();
let code = urlVars["oobCode"];
if (!code) {
code = "";
}

firebase.auth().verifyPasswordResetCode(code)
.then(function(email) {
$email.val(email);
}).catch(function(error) {
flash.error("Invalid password reset code. "
+ "The code may be malformed, expired, or has already been used.");
$submit.prop('disabled', true);
});

$password.keyup(function() {
$submit.prop('disabled', !checkPasswordValid($password.val()));
});
Expand All @@ -121,7 +73,6 @@
});

function selectPassword() {
let email = $email.val();
let pwd = $password.val();
if (pwd != $retype.val()) {
flash.clear();
Expand All @@ -135,31 +86,11 @@

// Disable the submit button so we only attempt once.
$submit.prop('disabled', true);

return firebase.auth().confirmPasswordReset(code, pwd)
.then(function() {
return true;
}).catch(function(error) {
flash.clear();
flash.error(error);
$submit.prop('disabled', false);
return false;
});
return true;
}

{{template "login/pwd-validate-js" .}}
});

function getUrlVars() {
let vars = [], hash;
let queryParams = window.location.href.slice(window.location.href.indexOf('?') + 1).split('&');
for (var i = 0; i < queryParams.length; i++) {
v = queryParams[i].split('=');
vars.push(v[0]);
vars[v[0]] = v[1];
}
return vars;
}
</script>
</body>

Expand Down
4 changes: 2 additions & 2 deletions cmd/server/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,8 @@ func realMain(ctx context.Context) error {
sub.Handle("/", loginController.HandleLogin()).Methods("GET")
sub.Handle("/login/reset-password", loginController.HandleShowResetPassword()).Methods("GET")
sub.Handle("/login/reset-password", loginController.HandleSubmitResetPassword()).Methods("POST")
sub.Handle("/login/select-password", loginController.HandleShowSelectNewPassword()).Methods("GET")
sub.Handle("/login/select-password", loginController.HandleSubmitNewPassword()).Methods("POST")
sub.Handle("/login/select-password", loginController.HandleShowSelectNewPassword()).Queries("oobCode", "").Methods("GET")
sub.Handle("/login/select-password", loginController.HandleSubmitNewPassword()).Queries("oobCode", "").Methods("POST")
sub.Handle("/session", loginController.HandleCreateSession()).Methods("POST")
sub.Handle("/signout", loginController.HandleSignOut()).Methods("GET")

Expand Down
12 changes: 11 additions & 1 deletion internal/firebase/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@

package firebase

import "errors"
import (
"errors"
)

var (
ErrEmailNotFound = &ErrorDetails{Err: "EMAIL_NOT_FOUND"}
ErrInvalidOOBCode = &ErrorDetails{Err: "INVALID_OOB_CODE"}
ErrExpiredOOBCode = &ErrorDetails{Err: "EXPIRED_OOB_CODE"}
ErrCredentialTooOld = &ErrorDetails{Err: "CREDENTIAL_TOO_OLD_LOGIN_AGAIN"}
ErrTokenExpired = &ErrorDetails{Err: "TOKEN_EXPIRED"}
ErrInvalidToken = &ErrorDetails{Err: "INVALID_ID_TOKEN"}
Expand All @@ -36,6 +39,13 @@ func (err *ErrorDetails) Error() string {
return err.Err
}

func (err *ErrorDetails) Is(target error) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you should need this? It also doesn't guarantee that the error is an ErrorDetails because it could be wrapped.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the static ones up top, but they don't match unless it's a complete match and I also specify the http code for them. I was hoping to match only on this error enum firebase gives back

if tErr, ok := target.(*ErrorDetails); ok {
return err.Err == tErr.Err
}
return false
}

// ShouldReauthenticate returns true for errors that require a refreshed auth token.
func (err *ErrorDetails) ShouldReauthenticate() bool {
return errors.Is(err, ErrCredentialTooOld) ||
Expand Down
44 changes: 31 additions & 13 deletions internal/firebase/verify_password_reset_code.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,38 +34,56 @@ type verifyPasswordResetCodeRequest struct {
// using the code.
//
// See: https://firebase.google.com/docs/reference/rest/auth#section-send-password-reset-email
func (c *Client) VerifyPasswordResetCode(ctx context.Context, code, newPassword string) error {
r := &verifyPasswordResetCodeRequest{
Code: code,
NewPassword: newPassword,
func (c *Client) VerifyPasswordResetCode(ctx context.Context, code string) (string, error) {
return c.ChangePasswordWithCode(ctx, code, "")
}

func (c *Client) ChangePasswordWithCode(ctx context.Context, code, newPassword string) (string, error) {
r := &verifyPasswordResetCodeRequest{Code: code}
if newPassword != "" {
r.NewPassword = newPassword
}

var body bytes.Buffer
if err := json.NewEncoder(&body).Encode(r); err != nil {
return fmt.Errorf("failed to create json body: %w", err)
return "", fmt.Errorf("failed to create json body: %w", err)
}

u := c.buildURL("/v1/accounts:resetPassword")
req, err := http.NewRequestWithContext(ctx, http.MethodPost, u, &body)
if err != nil {
return fmt.Errorf("failed to build request: %w", err)
return "", fmt.Errorf("failed to build request: %w", err)
}
req.Header.Add("Accept", "application/json")
req.Header.Add("Content-Type", "application/json")

resp, err := c.client.Do(req)
if err != nil {
return fmt.Errorf("failed to send password reset email: %w", err)
return "", fmt.Errorf("failed to send password reset email: %w", err)
}
defer resp.Body.Close()

if status := resp.StatusCode; status != http.StatusOK {
b, err := ioutil.ReadAll(resp.Body)
if err != nil {
return fmt.Errorf("response was %d, but failed to read body: %w", status, err)
status := resp.StatusCode
b, err := ioutil.ReadAll(resp.Body)
if err != nil {
return "", fmt.Errorf("response was %d, but failed to read body: %w", status, err)
}

if status != http.StatusOK {
// Try to unmarshal the error message. Firebase uses these as enum values to expand on the code.
var m map[string]ErrorDetails
if err := json.Unmarshal(b, &m); err == nil {
d := m["error"]
return "", &d
}
return fmt.Errorf("failure %d: %s", status, string(b))

return "", fmt.Errorf("failure %d: %s", status, string(b))
}

var m map[string]string
if err := json.Unmarshal(b, &m); err == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to handle the err case here?

Copy link
Contributor Author

@whaught whaught Sep 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still want this to be understood as success if we fail to parse the body of the response

return m["email"], nil
}

return nil
return "", nil
}
22 changes: 11 additions & 11 deletions pkg/controller/admin/admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,21 @@ import (
)

type Controller struct {
config *config.ServerConfig
db *database.Database
fbAuth *auth.Client
h *render.Renderer
logger *zap.SugaredLogger
config *config.ServerConfig
db *database.Database
firebaseAuth *auth.Client
h *render.Renderer
logger *zap.SugaredLogger
}

func New(ctx context.Context, config *config.ServerConfig, db *database.Database, fbAuth *auth.Client, h *render.Renderer) *Controller {
func New(ctx context.Context, config *config.ServerConfig, db *database.Database, firebaseAuth *auth.Client, h *render.Renderer) *Controller {
logger := logging.FromContext(ctx).Named("admin")

return &Controller{
config: config,
db: db,
fbAuth: fbAuth,
h: h,
logger: logger,
config: config,
db: db,
firebaseAuth: firebaseAuth,
h: h,
logger: logger,
}
}
2 changes: 1 addition & 1 deletion pkg/controller/admin/users.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (c *Controller) HandleUsersCreate() http.Handler {
return
}

created, err := user.CreateFirebaseUser(ctx, c.fbAuth)
created, err := user.CreateFirebaseUser(ctx, c.firebaseAuth)
if err != nil {
flash.Alert("Failed to create user: %v", err)
c.renderNewUser(ctx, w, user)
Expand Down
Loading