Skip to content

feat(oidc): more detailed error messages and error validation #2944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
88 changes: 52 additions & 36 deletions api/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,12 +614,13 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) {
loginURL, _ := url.JoinPath(util.Config.WebHost, "auth/login")

if err != nil {
log.Error(err.Error())
log.Errorf("Failed to retrieve OAuth state cookie: %v", err)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

if r.FormValue("state") != oauthState.Value {
log.Warn("OAuth state mismatch")
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}
Expand All @@ -628,54 +629,68 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) {

_oidc, oauth, err := getOidcProvider(pid, ctx, r.URL.Path)
if err != nil {
log.Error(err.Error())
log.Errorf("Failed to get OIDC provider: %v", err)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

provider, ok := util.Config.OidcProviders[pid]
if !ok {
log.Error(fmt.Errorf("no such provider: %s", pid))
log.Errorf("No such OIDC provider: %s", pid)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

verifier := _oidc.Verifier(&oidc.Config{ClientID: oauth.ClientID})

code := r.URL.Query().Get("code")
if code == "" {
log.Warn("Missing authorization code in request")
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

oauth2Token, err := oauth.Exchange(ctx, code)
if err != nil {
log.Error(err.Error())
log.Errorf("Failed to exchange authorization code: %v", err)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

var claims claimResult

// Extract the ID Token from OAuth2 token.
rawIDToken, ok := oauth2Token.Extra("id_token").(string)

if ok && rawIDToken != "" {
var idToken *oidc.IDToken
// Parse and verify ID Token payload.
idToken, err = verifier.Verify(ctx, rawIDToken)

if err == nil {
claims, err = claimOidcToken(idToken, provider)
idToken, err2 := verifier.Verify(ctx, rawIDToken)
if err2 != nil {
log.Errorf("Failed to verify ID token: %v", err2)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}
claims, err2 = claimOidcToken(idToken, provider)
if err2 != nil {
log.Errorf("Failed to parse ID token claims: %v", err2)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}
} else {
var userInfo *oidc.UserInfo
userInfo, err = _oidc.UserInfo(ctx, oauth2.StaticTokenSource(oauth2Token))

if err == nil {
userInfo, err2 := _oidc.UserInfo(ctx, oauth2.StaticTokenSource(oauth2Token))
if err2 != nil {
log.Errorf("Failed to retrieve user info: %v", err2)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

if userInfo.Email == "" {
claims, err = claimOidcUserInfo(userInfo, provider)
} else {
claims.email = userInfo.Email
claims.name = userInfo.Profile
if userInfo.Email == "" {
claims, err2 = claimOidcUserInfo(userInfo, provider)
if err2 != nil {
log.Errorf("Failed to parse user info claims: %v", err2)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}
} else {
claims.email = userInfo.Email
claims.name = userInfo.Profile
}

claims.username = getRandomUsername()
Expand All @@ -684,41 +699,42 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) {
}
}

if err != nil {
log.Error(err.Error())
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}
user, err := helpers.Store(r).GetUserByLoginOrEmail("", claims.email)

if errors.Is(err, db.ErrNotFound) {
if util.Config.OidcProviders[pid].DisableUserCreation {
log.Errorf("OIDC user '%s' not found and user creation is disabled", claims.email)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

user, err := helpers.Store(r).GetUserByLoginOrEmail("", claims.email) // ignore username because it creates a lot of problems
if err != nil {
user = db.User{
Username: claims.username,
Name: claims.name,
Email: claims.email,
External: true,
}
user, err = helpers.Store(r).CreateUserWithoutPassword(user)
if err != nil {
log.Error(err.Error())
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}
}

if err != nil {
log.Errorf("Failed to create or retrieve user: %v", err)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

if !user.External {
log.Error(fmt.Errorf("OIDC user '%s' conflicts with local user", user.Username))
log.Errorf("OIDC user '%s' conflicts with local user", user.Username)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}

createSession(w, r, user)

redirectPath := mux.Vars(r)["redirect_path"]

redirectPath, err = url.JoinPath(util.Config.WebHost, redirectPath)
if err != nil {
log.Error(err)
log.Errorf("Failed to construct redirect path: %v", err)
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}
Expand Down
2 changes: 2 additions & 0 deletions util/OdbcProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ type OidcProvider struct {
NameClaim string `json:"name_claim" default:"preferred_username"`
EmailClaim string `json:"email_claim" default:"email"`
Order int `json:"order"`

DisableUserCreation bool `json:"disable_user_creation"`
}

type ClaimsProvider interface {
Expand Down
Loading