Skip to content

Commit

Permalink
auth: Redirect github oauth login attempt back if user cancels (#23083)
Browse files Browse the repository at this point in the history
Before, we would show a raw plaintext error. Instead, we detect the
specifc `access_denied` error returned from GitHub and redirect the user
back.
  • Loading branch information
ryanslade committed Jul 21, 2021
1 parent 6fa3751 commit 1a3e7ad
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
31 changes: 30 additions & 1 deletion enterprise/cmd/frontend/internal/auth/githuboauth/provider.go
Expand Up @@ -5,9 +5,13 @@ import (
"net/http"
"net/url"

"github.com/dghubble/gologin"
"github.com/dghubble/gologin/github"
goauth2 "github.com/dghubble/gologin/oauth2"
"github.com/inconshreveable/log15"
"golang.org/x/oauth2"

"github.com/sourcegraph/sourcegraph/cmd/frontend/auth"
"github.com/sourcegraph/sourcegraph/cmd/frontend/envvar"
"github.com/sourcegraph/sourcegraph/enterprise/cmd/frontend/internal/auth/oauth"
"github.com/sourcegraph/sourcegraph/internal/database/dbutil"
Expand Down Expand Up @@ -66,12 +70,37 @@ func parseProvider(p *schema.GitHubAuthProvider, db dbutil.DB, sourceCfg schema.
allowSignup: p.AllowSignup,
allowOrgs: p.AllowOrgs,
}, sessionKey),
nil,
http.HandlerFunc(failureHandler),
)
},
}), messages
}

func failureHandler(w http.ResponseWriter, r *http.Request) {
// As a special case wa want to handle `access_denied` errors by redirecting
// back. This case arises when the user decides not to proceed by clicking `cancel`.
if err := r.URL.Query().Get("error"); err != "access_denied" {
// Fall back to default failure handler
gologin.DefaultFailureHandler.ServeHTTP(w, r)
return
}

ctx := r.Context()
encodedState, err := goauth2.StateFromContext(ctx)
if err != nil {
log15.Error("OAuth failed: could not get state from context.", "error", err)
http.Error(w, "Authentication failed. Try signing in again (and clearing cookies for the current site). The error was: could not get OAuth state from context.", http.StatusInternalServerError)
return
}
state, err := oauth.DecodeState(encodedState)
if err != nil {
log15.Error("OAuth failed: could not decode state.", "error", err)
http.Error(w, "Authentication failed. Try signing in again (and clearing cookies for the current site). The error was: could not get decode OAuth state.", http.StatusInternalServerError)
return
}
http.Redirect(w, r, auth.SafeRedirectURL(state.Redirect), http.StatusFound)
}

var clientIDSecretValidator = lazyregexp.New("^[a-z0-9]*$")

func validateClientIDAndSecret(clientIDOrSecret string) (valid bool) {
Expand Down
2 changes: 1 addition & 1 deletion enterprise/cmd/frontend/internal/auth/oauth/session.go
Expand Up @@ -67,7 +67,7 @@ func SessionIssuer(s SessionIssuerHelper, sessionKey string) http.Handler {
return
}

// Delete state cookie (no longer needed, while be stale if user logs out and logs back in within 120s)
// Delete state cookie (no longer needed, will be stale if user logs out and logs back in within 120s)
defer s.DeleteStateCookie(w)

if state.Op == LoginStateOpCreateCodeHostConnection {
Expand Down

0 comments on commit 1a3e7ad

Please sign in to comment.