Skip to content

Commit

Permalink
refactor: return error earlier
Browse files Browse the repository at this point in the history
  • Loading branch information
kangmingtay committed Nov 13, 2023
1 parent f93d98a commit 53cce71
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 15 deletions.
29 changes: 15 additions & 14 deletions internal/api/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ func (a *API) ExternalProviderCallback(w http.ResponseWriter, r *http.Request) e
return nil
}

// errReturnNil is a hack that signals internalExternalProviderCallback to return nil
var errReturnNil = errors.New("createAccountFromExternalIdentity: return nil in internalExternalProviderCallback")
var errEmailVerificationRequired = errors.New("email verification required")

func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Request) error {
ctx := r.Context()
Expand Down Expand Up @@ -196,10 +195,6 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
}
} else {
if user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType); terr != nil {
if errors.Is(terr, errReturnNil) {
return nil
}

return terr
}
}
Expand All @@ -218,11 +213,23 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
}
return nil
})

rurl := a.getExternalRedirectURL(r)
if err != nil {
if errors.Is(err, errEmailVerificationRequired) {
flowType := models.ImplicitFlow
if flowState != nil {
flowType = models.PKCEFlow
}
rurl, err = a.prepErrorRedirectURL(unauthorizedError("Unverified email with %v. Verify the email with %v in order to sign in", providerType, providerType), w, r, rurl, flowType)
if err != nil {
return err
}
http.Redirect(w, r, rurl, http.StatusFound)
}
return err
}

rurl := a.getExternalRedirectURL(r)
if flowState != nil {
// This means that the callback is using PKCE
// Set the flowState.AuthCode to the query param here
Expand All @@ -244,12 +251,6 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
if err := a.setCookieTokens(config, token, false, w); err != nil {
return internalServerError("Failed to set JWT cookie. %s", err)
}
} else {
// Left as hash fragment to comply with spec. Additionally, may override existing error query param if set to PKCE.
rurl, err = a.prepErrorRedirectURL(unauthorizedError("Unverified email with %v. A confirmation email has been sent to your %v email.", providerType, providerType), w, r, rurl, models.ImplicitFlow)
if err != nil {
return err
}
}

http.Redirect(w, r, rurl, http.StatusFound)
Expand Down Expand Up @@ -385,7 +386,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.
}
if !config.Mailer.AllowUnverifiedEmailSignIns {
// email must be verified to issue a token
return nil, errReturnNil
return nil, errEmailVerificationRequired
}
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion internal/api/token_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R

user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType)
if terr != nil {
if errors.Is(terr, errReturnNil) {
if errors.Is(terr, errEmailVerificationRequired) {
return nil
}

Expand Down
2 changes: 2 additions & 0 deletions internal/api/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,10 @@ func (a *API) prepErrorRedirectURL(err *HTTPError, w http.ResponseWriter, r *htt
q.Set("error_code", strconv.Itoa(err.Code))
q.Set("error_description", err.Message)
if flowType == models.PKCEFlow {
// Additionally, may override existing error query param if set to PKCE.
u.RawQuery = q.Encode()
}
// Left as hash fragment to comply with spec.
u.Fragment = hq.Encode()
return u.String(), nil
}
Expand Down

0 comments on commit 53cce71

Please sign in to comment.