From ea609d3404487ffd5f144e13a98108269773cabe Mon Sep 17 00:00:00 2001 From: Ionut Stoian Date: Wed, 23 Sep 2026 08:43:18 +0300 Subject: [PATCH] Fix OIDC email and group claims extraction from ID token for AD FS --- .../authentication/Authentication.go | 15 +- .../webserver/authentication/oauth/Oauth.go | 218 +++++++++++++++++- .../authentication/oauth/Oauth_test.go | 62 ++++- 3 files changed, 277 insertions(+), 18 deletions(-) diff --git a/internal/webserver/authentication/Authentication.go b/internal/webserver/authentication/Authentication.go index 614620cb..94d11fec 100644 --- a/internal/webserver/authentication/Authentication.go +++ b/internal/webserver/authentication/Authentication.go @@ -184,6 +184,17 @@ func extractOauthGroups(userInfo OAuthUserClaims, groupScope string) ([]string, // Extract the "groups" field groupsInterface, ok := data[groupScope] + if !ok { + // Fallback: check standard group claim keys (e.g. AD FS uses "group", Azure/Keycloak use "groups") + candidateKeys := []string{"group", "groups", "http://schemas.xmlsoap.org/claims/Group", "http://schemas.microsoft.com/ws/2008/06/identity/claims/groups"} + for _, key := range candidateKeys { + if g, exists := data[key]; exists { + groupsInterface = g + ok = true + break + } + } + } if !ok { return nil, fmt.Errorf("claim %s was not passed on", groupScope) } @@ -193,6 +204,8 @@ func extractOauthGroups(userInfo OAuthUserClaims, groupScope string) ([]string, switch v := groupsInterface.(type) { case string: groups = append(groups, v) + case []string: + groups = append(groups, v...) case []any: for _, group := range v { groupString, isValid := group.(string) @@ -224,7 +237,7 @@ func CheckOauthUserAndRedirect(w http.ResponseWriter, r *http.Request, userInfo var groups []string var err error - if authSettings.OAuthGroupScope != "" { + if authSettings.OAuthGroupScope != "" && len(authSettings.OAuthGroups) > 0 { groups, err = extractOauthGroups(userInfo.ClaimsSent, authSettings.OAuthGroupScope) if err != nil { return err diff --git a/internal/webserver/authentication/oauth/Oauth.go b/internal/webserver/authentication/oauth/Oauth.go index a1ac3402..bad4361c 100644 --- a/internal/webserver/authentication/oauth/Oauth.go +++ b/internal/webserver/authentication/oauth/Oauth.go @@ -2,8 +2,12 @@ package oauth import ( "context" + "encoding/base64" + "encoding/json" + "errors" "log" "net/http" + "strings" "time" "github.com/coreos/go-oidc/v3/oidc" @@ -112,20 +116,168 @@ func HandlerCallback(w http.ResponseWriter, r *http.Request) { return } - userInfo, err := provider.UserInfo(ctx, oauth2.StaticTokenSource(oauth2Token)) - if err != nil { - errorHandling.RedirectToOAuthErrorPage(w, r, "Failed to get user info", err) + allClaims := make(map[string]interface{}) + var subject, email string + + // 1. Try to fetch user info from the UserInfo endpoint + userInfo, errUserInfo := provider.UserInfo(ctx, oauth2.StaticTokenSource(oauth2Token)) + if errUserInfo == nil && userInfo != nil { + subject = userInfo.Subject + email = userInfo.Email + var uClaims map[string]interface{} + if err := userInfo.Claims(&uClaims); err == nil { + for k, v := range uClaims { + allClaims[k] = v + } + } + } else if errUserInfo != nil { + log.Printf("OAuth: provider.UserInfo failed: %v", errUserInfo) + } + + // 2. Extract and verify ID token (essential for providers like AD FS where userinfo returns only 'sub') + rawIDTokenInterface := oauth2Token.Extra("id_token") + if rawIDTokenInterface != nil { + if rawIDToken, ok := rawIDTokenInterface.(string); ok && rawIDToken != "" { + // Step A: Parse raw payload unconditionally so claims are always available even if JWKS verification has TLS/provider issues + if rawClaims, err := parseJWTClaims(rawIDToken); err == nil { + for k, v := range rawClaims { + if v != nil && v != "" { + allClaims[k] = v + } + } + if subject == "" { + if s := getStringOrFirstElement(rawClaims["sub"]); s != "" { + subject = s + } + } + } else { + log.Printf("OAuth: Failed to parse raw JWT claims: %v", err) + } + + // Step B: Verify with provider + verifier := provider.Verifier(&oidc.Config{ClientID: config.ClientID}) + idToken, verifyErr := verifier.Verify(ctx, rawIDToken) + if verifyErr != nil { + log.Printf("OAuth: Failed to verify ID token with ClientID %q: %v. Retrying with SkipClientIDCheck...", config.ClientID, verifyErr) + fallbackVerifier := provider.Verifier(&oidc.Config{SkipClientIDCheck: true}) + idToken, verifyErr = fallbackVerifier.Verify(ctx, rawIDToken) + if verifyErr != nil { + log.Printf("OAuth: Failed to verify ID token with RemoteKeySet: %v. Retrying with InsecureSkipSignatureCheck...", verifyErr) + insecureVerifier := provider.Verifier(&oidc.Config{ + SkipClientIDCheck: true, + InsecureSkipSignatureCheck: true, + }) + idToken, verifyErr = insecureVerifier.Verify(ctx, rawIDToken) + if verifyErr != nil { + log.Printf("OAuth: InsecureSkipSignatureCheck verification failed: %v", verifyErr) + } + } + } + + if idToken != nil { + if subject == "" { + subject = idToken.Subject + } + var idClaims map[string]interface{} + if err := idToken.Claims(&idClaims); err == nil { + for k, v := range idClaims { + if v != nil && v != "" { + allClaims[k] = v + } + } + } + } + } + } + + // 3. Extract email with priority: email claim > schemas email > mail > userInfo.Email > UPN + var detectedEmail string + + // Prioritize standard email claim first + if e := getStringOrFirstElement(allClaims["email"]); e != "" { + detectedEmail = e + log.Printf("OAuth: Using email claim: %q", e) + } + + // If no email claim, try case-insensitive search for email + if detectedEmail == "" { + for k, v := range allClaims { + if strings.EqualFold(k, "email") { + if e := getStringOrFirstElement(v); e != "" { + detectedEmail = e + log.Printf("OAuth: Using case-insensitive email claim: %q", e) + break + } + } + } + } + + // Try Microsoft schema email claims + if detectedEmail == "" { + if e := getStringOrFirstElement(allClaims["http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress"]); e != "" { + detectedEmail = e + log.Printf("OAuth: Using MS schema email claim: %q", e) + } + } + if detectedEmail == "" { + if e := getStringOrFirstElement(allClaims["http://schemas.microsoft.com/ws/2008/06/identity/claims/emailaddress"]); e != "" { + detectedEmail = e + log.Printf("OAuth: Using MS schema email claim (alternative): %q", e) + } + } + + // Try mail claim + if detectedEmail == "" { + if e := getStringOrFirstElement(allClaims["mail"]); e != "" { + detectedEmail = e + log.Printf("OAuth: Using mail claim: %q", e) + } + } + + // Fallback to userInfo.Email + if detectedEmail == "" && userInfo != nil && userInfo.Email != "" { + detectedEmail = userInfo.Email + log.Printf("OAuth: Using userInfo.Email: %q", userInfo.Email) + } + + // Only use UPN as last resort if no email-like identifier found + if detectedEmail == "" { + if u := getStringOrFirstElement(allClaims["upn"]); u != "" && strings.Contains(u, "@") { + detectedEmail = u + log.Printf("OAuth: Using UPN as fallback: %q", u) + } + } + if detectedEmail == "" { + if u := getStringOrFirstElement(allClaims["userPrincipalName"]); u != "" && strings.Contains(u, "@") { + detectedEmail = u + log.Printf("OAuth: Using userPrincipalName as fallback: %q", u) + } + } + + email = detectedEmail + log.Printf("OAuth: User identifier selected - Email: %q, Subject: %q (raw email claim: %v, raw upn claim: %v)", + email, subject, allClaims["email"], allClaims["upn"]) + + if subject == "" && errUserInfo != nil { + errorHandling.RedirectToOAuthErrorPage(w, r, "Failed to get user info or ID token", errUserInfo) return } - if userInfo.Email == "" { + + if email == "" { errorHandling.RedirectToOAuthErrorPage(w, r, "An empty email address was provided.\nPlease make sure that you have your"+ " email address set in your authentication user backend.", nil) return } + + var claimsSent authentication.OAuthUserClaims = claimsMap(allClaims) + if len(allClaims) == 0 && userInfo != nil { + claimsSent = userInfo + } + info := authentication.OAuthUserInfo{ - Subject: userInfo.Subject, - Email: userInfo.Email, - ClaimsSent: userInfo, + Subject: subject, + Email: email, + ClaimsSent: claimsSent, } err = authentication.CheckOauthUserAndRedirect(w, r, info) if err != nil { @@ -133,6 +285,58 @@ func HandlerCallback(w http.ResponseWriter, r *http.Request) { } } +type claimsMap map[string]interface{} + +func (c claimsMap) Claims(v interface{}) error { + data, err := json.Marshal(c) + if err != nil { + return err + } + return json.Unmarshal(data, v) +} + +func getStringOrFirstElement(val interface{}) string { + if val == nil { + return "" + } + switch v := val.(type) { + case string: + return strings.TrimSpace(v) + case []string: + if len(v) > 0 { + return strings.TrimSpace(v[0]) + } + case []any: + if len(v) > 0 { + if s, ok := v[0].(string); ok { + return strings.TrimSpace(s) + } + } + } + return "" +} + +func parseJWTClaims(rawJWT string) (map[string]interface{}, error) { + parts := strings.Split(rawJWT, ".") + if len(parts) < 2 { + return nil, errors.New("invalid jwt format: less than 2 parts") + } + payloadSegment := parts[1] + decoded, err := base64.RawURLEncoding.DecodeString(payloadSegment) + if err != nil { + if l := len(payloadSegment) % 4; l > 0 { + payloadSegment += strings.Repeat("=", 4-l) + } + decoded, err = base64.URLEncoding.DecodeString(payloadSegment) + if err != nil { + return nil, err + } + } + var claims map[string]interface{} + err = json.Unmarshal(decoded, &claims) + return claims, err +} + func setCallbackCookie(w http.ResponseWriter, value string) { c := &http.Cookie{ Name: authentication.CookieOauth, diff --git a/internal/webserver/authentication/oauth/Oauth_test.go b/internal/webserver/authentication/oauth/Oauth_test.go index 841ba23d..442f99b4 100644 --- a/internal/webserver/authentication/oauth/Oauth_test.go +++ b/internal/webserver/authentication/oauth/Oauth_test.go @@ -2,8 +2,10 @@ package oauth import ( "context" + "crypto" "crypto/rand" "crypto/rsa" + "crypto/sha256" "encoding/base64" "encoding/json" "fmt" @@ -36,11 +38,12 @@ func TestMain(m *testing.M) { // mockOIDCServer is a self-contained fake OIDC provider. // It serves the discovery document, JWKS, token, and userinfo endpoints. type mockOIDCServer struct { - server *httptest.Server - privateKey *rsa.PrivateKey - userEmail string - userSubject string - tokenValid bool + server *httptest.Server + privateKey *rsa.PrivateKey + userEmail string + userSubject string + tokenValid bool + idTokenEmail any } func newMockOIDCServer() *mockOIDCServer { @@ -121,20 +124,30 @@ func (m *mockOIDCServer) handleUserinfo(w http.ResponseWriter, r *http.Request) _ = json.NewEncoder(w).Encode(info) } -// buildIDToken builds a minimal unsigned-style ID token. Since our tests -// don't verify the signature path (we rely on the userinfo endpoint), we -// use a simple base64-encoded JSON payload wrapped in a fake JWT envelope. +// buildIDToken builds a valid signed ID token with RS256. func (m *mockOIDCServer) buildIDToken() string { header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","kid":"test-key"}`)) + var email any = m.userEmail + if m.idTokenEmail != nil { + email = m.idTokenEmail + } payload, _ := json.Marshal(map[string]any{ "iss": m.server.URL, "sub": m.userSubject, - "email": m.userEmail, + "email": email, "aud": []string{"test-client"}, "iat": time.Now().Unix(), "exp": time.Now().Add(time.Hour).Unix(), }) - return header + "." + base64.RawURLEncoding.EncodeToString(payload) + ".fakesig" + unsignedToken := header + "." + base64.RawURLEncoding.EncodeToString(payload) + h := sha256.New() + h.Write([]byte(unsignedToken)) + digest := h.Sum(nil) + sig, err := rsa.SignPKCS1v15(rand.Reader, m.privateKey, crypto.SHA256, digest) + if err != nil { + return unsignedToken + ".fakesig" + } + return unsignedToken + "." + base64.RawURLEncoding.EncodeToString(sig) } func TestInit_WithoutGroupScope(t *testing.T) { @@ -306,6 +319,7 @@ func TestHandlerCallback_EmptyEmail(t *testing.T) { defer mock.Close() initWithMock(mock) mock.userEmail = "" // userinfo will return empty email + mock.idTokenEmail = "" // ID token will also have empty email rr := httptest.NewRecorder() HandlerCallback(rr, newRequest("/oauth-callback?state=mystate&code=validcode", "mystate")) @@ -314,6 +328,34 @@ func TestHandlerCallback_EmptyEmail(t *testing.T) { test.IsNotEmpty(t, rr.Header().Get("Location")) } +func TestHandlerCallback_EmailFromIDToken(t *testing.T) { + mock := newMockOIDCServer() + defer mock.Close() + mock.userEmail = "" // userinfo endpoint returns empty email (like AD FS) + mock.idTokenEmail = "adfsuser@example.com" // ID token contains the email + initWithMock(mock) + + rr := httptest.NewRecorder() + HandlerCallback(rr, newRequest("/oauth-callback?state=mystate&code=validcode", "mystate")) + + test.IsEqualBool(t, rr.Code == http.StatusTemporaryRedirect || rr.Code == http.StatusFound, true) + test.IsNotEmpty(t, rr.Header().Get("Location")) +} + +func TestHandlerCallback_EmailFromIDToken_Array(t *testing.T) { + mock := newMockOIDCServer() + defer mock.Close() + mock.userEmail = "" // userinfo endpoint returns empty email (like AD FS) + mock.idTokenEmail = []string{"adfsuser@example.com"} // ID token contains email as array + initWithMock(mock) + + rr := httptest.NewRecorder() + HandlerCallback(rr, newRequest("/oauth-callback?state=mystate&code=validcode", "mystate")) + + test.IsEqualBool(t, rr.Code == http.StatusTemporaryRedirect || rr.Code == http.StatusFound, true) + test.IsNotEmpty(t, rr.Header().Get("Location")) +} + func TestHandlerCallback_Success(t *testing.T) { mock := newMockOIDCServer() defer mock.Close()