diff --git a/config/oidc_metadata.go b/config/oidc_metadata.go index 02f02277d..c18509270 100644 --- a/config/oidc_metadata.go +++ b/config/oidc_metadata.go @@ -74,19 +74,27 @@ func getMetadata() { } oidcMetadata = metadata - // We don't check if the endpoint(s) are set. Just overwrite to ensure - // our default values are not being used if the issuer is not CILogon - if err := param.Set("OIDC.DeviceAuthEndpoint", metadata.DeviceAuthURL); err != nil { - log.WithError(err).Warn("Failed to set OIDC.DeviceAuthEndpoint from issuer metadata") + // Only set endpoints from metadata if they were not explicitly configured by the user. + // This allows users to override discovery with explicit endpoint URLs (e.g., for GitHub OAuth2) + if !param.OIDC_DeviceAuthEndpoint.IsSet() { + if err := param.Set("OIDC.DeviceAuthEndpoint", metadata.DeviceAuthURL); err != nil { + log.WithError(err).Warn("Failed to set OIDC.DeviceAuthEndpoint from issuer metadata") + } } - if err := param.Set("OIDC.TokenEndpoint", metadata.TokenURL); err != nil { - log.WithError(err).Warn("Failed to set OIDC.TokenEndpoint from issuer metadata") + if !param.OIDC_TokenEndpoint.IsSet() { + if err := param.Set("OIDC.TokenEndpoint", metadata.TokenURL); err != nil { + log.WithError(err).Warn("Failed to set OIDC.TokenEndpoint from issuer metadata") + } } - if err := param.Set("OIDC.UserInfoEndpoint", metadata.UserInfoURL); err != nil { - log.WithError(err).Warn("Failed to set OIDC.UserInfoEndpoint from issuer metadata") + if !param.OIDC_UserInfoEndpoint.IsSet() { + if err := param.Set("OIDC.UserInfoEndpoint", metadata.UserInfoURL); err != nil { + log.WithError(err).Warn("Failed to set OIDC.UserInfoEndpoint from issuer metadata") + } } - if err := param.Set("OIDC.AuthorizationEndpoint", metadata.AuthURL); err != nil { - log.WithError(err).Warn("Failed to set OIDC.AuthorizationEndpoint from issuer metadata") + if !param.OIDC_AuthorizationEndpoint.IsSet() { + if err := param.Set("OIDC.AuthorizationEndpoint", metadata.AuthURL); err != nil { + log.WithError(err).Warn("Failed to set OIDC.AuthorizationEndpoint from issuer metadata") + } } } @@ -170,12 +178,21 @@ func GetOIDCAuthorizationEndpoint() (result string, err error) { func GetOIDCSupportedScopes() (results []string, err error) { onceMetadata.Do(getMetadata) - err = metadataError - if err != nil { + // First check if we have scopes from OIDC discovery metadata + if metadataError == nil && len(oidcMetadata.ScopesSupported) > 0 { + results = make([]string, len(oidcMetadata.ScopesSupported)) + copy(results, oidcMetadata.ScopesSupported) + return + } + // Fall back to explicitly configured OIDC.Scopes (e.g., for OAuth2-only providers like GitHub) + configuredScopes := param.OIDC_Scopes.GetStringSlice() + if len(configuredScopes) > 0 { + results = make([]string, len(configuredScopes)) + copy(results, configuredScopes) return } - results = make([]string, len(oidcMetadata.ScopesSupported)) - copy(results, oidcMetadata.ScopesSupported) + // If neither metadata nor explicit config, return the metadata error + err = metadataError return } diff --git a/config/oidc_metadata_test.go b/config/oidc_metadata_test.go index 92072a4e3..1fde216ff 100644 --- a/config/oidc_metadata_test.go +++ b/config/oidc_metadata_test.go @@ -19,6 +19,7 @@ package config import ( + "sync" "testing" "github.com/stretchr/testify/assert" @@ -85,3 +86,58 @@ func TestGetOIDCProvider(t *testing.T) { assert.Equal(t, Globus, get) }) } + +func TestGetMetadataRespectsExplicitEndpoints(t *testing.T) { + t.Cleanup(func() { + ResetConfig() + // Note: Resetting sync.Once in tests is generally not recommended due to potential race conditions. + // However, in this case, we run tests sequentially and need to re-trigger metadata discovery. + // In production code, sync.Once ensures getMetadata() is only called once per process lifetime. + onceMetadata = sync.Once{} + metadataError = nil + oidcMetadata = nil + }) + + t.Run("explicit-endpoints-not-overridden-by-issuer", func(t *testing.T) { + ResetConfig() + // Reset the sync.Once so we can test getMetadata again in this isolated test + onceMetadata = sync.Once{} + metadataError = nil + oidcMetadata = nil + + // Set explicit endpoints (e.g., for GitHub OAuth2) + explicitAuthEndpoint := "https://github.com/login/oauth/authorize" + explicitTokenEndpoint := "https://github.com/login/oauth/access_token" + explicitUserInfoEndpoint := "https://api.github.com/user" + explicitDeviceAuthEndpoint := "https://github.com/login/device/code" + + require.NoError(t, param.Set(param.OIDC_AuthorizationEndpoint.GetName(), explicitAuthEndpoint)) + require.NoError(t, param.Set(param.OIDC_TokenEndpoint.GetName(), explicitTokenEndpoint)) + require.NoError(t, param.Set(param.OIDC_UserInfoEndpoint.GetName(), explicitUserInfoEndpoint)) + require.NoError(t, param.Set(param.OIDC_DeviceAuthEndpoint.GetName(), explicitDeviceAuthEndpoint)) + + // Set OIDC.Issuer to CILogon (which has OIDC discovery) + // This should NOT override the explicitly set endpoints + require.NoError(t, param.Set(param.OIDC_Issuer.GetName(), "https://cilogon.org")) + + // Call the metadata discovery - it will try to fetch from CILogon but should not override + onceMetadata.Do(getMetadata) + + // Verify the endpoints are still what we set explicitly, not CILogon's + authEndpoint, err := GetOIDCAuthorizationEndpoint() + require.NoError(t, err) + assert.Equal(t, explicitAuthEndpoint, authEndpoint, "Authorization endpoint should not be overridden") + + tokenEndpoint, err := GetOIDCTokenEndpoint() + require.NoError(t, err) + assert.Equal(t, explicitTokenEndpoint, tokenEndpoint, "Token endpoint should not be overridden") + + userInfoEndpoint, err := GetOIDCUserInfoEndpoint() + require.NoError(t, err) + assert.Equal(t, explicitUserInfoEndpoint, userInfoEndpoint, "UserInfo endpoint should not be overridden") + + deviceAuthEndpoint, err := GetOIDCDeviceAuthEndpoint() + require.NoError(t, err) + assert.Equal(t, explicitDeviceAuthEndpoint, deviceAuthEndpoint, "DeviceAuth endpoint should not be overridden") + }) +} diff --git a/docs/github-oauth-config-example.yaml b/docs/github-oauth-config-example.yaml new file mode 100644 index 000000000..b5244b0ba --- /dev/null +++ b/docs/github-oauth-config-example.yaml @@ -0,0 +1,73 @@ +# Example configuration addition for using GitHub OAuth2 with Pelican +# +# GitHub uses OAuth2 (not OIDC), so you need to explicitly configure +# the endpoints and claim names. This example shows how to set up +# Pelican to authenticate users via GitHub. +# +# Prerequisites: +# 1. Create a GitHub OAuth App at https://github.com/settings/developers +# 2. Set the callback URL to: https://your-server.example.com/api/v1.0/auth/oauth/callback +# 3. Note your Client ID and Client Secret + +OIDC: + # Set the issuer to GitHub (used as fallback for issuer claim) + Issuer: https://github.com + + # Explicitly configure OAuth2 endpoints (GitHub doesn't support OIDC discovery) + AuthorizationEndpoint: https://github.com/login/oauth/authorize + TokenEndpoint: https://github.com/login/oauth/access_token + UserInfoEndpoint: https://api.github.com/user + # GitHub supports device auth; this endpoint is required for `Origin.EnableIssuer: true` + DeviceAuthEndpoint: https://github.com/login/device/code + + # Your GitHub OAuth App credentials + ClientID: your-github-client-id + ClientSecretFile: /path/to/client-secret-file + + # GitHub-specific scopes (adjust as needed) + Scopes: + - user + - read:org + + # In dev (containers/tunnels), set this to the hostname the OAuth provider will redirect to (e.g., localhost:port) + ClientRedirectHostname: + +Issuer: + # Configure which claims to use from GitHub's user info response + # GitHub returns "login" for username instead of the OIDC standard "sub" + OIDCAuthenticationUserClaim: login + + # GitHub returns "id" (numeric) instead of the OIDC standard "sub" + OIDCSubjectClaim: id + + # GitHub doesn't return an "iss" claim, so this setting tells Pelican + # to fall back to OIDC.Issuer (configured above) + OIDCIssuerClaim: iss + + # Use GitHub organization membership as groups + # Requires the "read:org" scope (configured above) + GroupSource: github + + # Optional: Require users to be a member of at least one of these GitHub orgs + # If not specified, any authenticated GitHub user can access the server + GroupRequirements: + - my-github-org + - another-allowed-org + + # Define authorization based on GitHub organization membership + # This example grants read access to members of specific organizations + AuthorizationTemplates: + - actions: ["read", "create", "modify"] + prefix: /projects/foo + groups: ["my-github-org"] + # Members of "my-github-org" can read/create/modify objects in /projects/foo + + +Server: + ExternalWebUrl: https://your-server.example.com + # If you want to have the admin access to web UI after logging in via GitHub, set: + UIAdminUsers: [""] + +Origin: + EnableIssuer: true + EnableOIDC: true diff --git a/docs/parameters.yaml b/docs/parameters.yaml index 5ad064b23..794ad7462 100644 --- a/docs/parameters.yaml +++ b/docs/parameters.yaml @@ -2738,6 +2738,28 @@ type: string default: sub components: ["origin"] --- +name: Issuer.OIDCSubjectClaim +description: |+ + The claim to be used as the unique subject identifier for the user. + For OIDC providers, this is typically "sub". + For OAuth2 providers like GitHub, this might be "id". + + If the claim is not found in the user info response, the system will fall back to using the username. + If the claim value is numeric, it will be converted to a string. +type: string +default: sub +components: ["origin", "registry", "cache", "director"] +--- +name: Issuer.OIDCIssuerClaim +description: |+ + The claim to be used to identify the authentication provider (issuer). + For OIDC providers, this is typically "iss". + For OAuth2 providers that don't provide an issuer claim, the system will fall back to using + the value of OIDC.Issuer or the hostname from OIDC.AuthorizationEndpoint. +type: string +default: iss +components: ["origin", "registry", "cache", "director"] +--- name: Issuer.UserStripDomain description: |+ Some OIDC issuers generate a username of the form user@domain (such as `john.doe@gmail.com`); when @@ -2759,6 +2781,8 @@ description: |+ the value of the claim specified by `Issuer.OIDCGroupClaim` (defaults to "groups") as a list of groups. The value may either be a comma-separated string or an array of strings. - `internal`: Take group information from the Pelican server's internal user database. + - `github`: Fetch group information from GitHub organization. Each GitHub organization the user + belongs to becomes a group. Requires the OAuth2 application to have the `read:org` scope. type: string default: none components: ["origin"] @@ -2950,8 +2974,12 @@ description: |+ If the OIDC auto-discovery failed, Pelican will fall back to use individual endpoints set in the configuration. For any unset endpoints, Pelican will use default values, which are from CILogon. + **Note**: If you explicitly set the OIDC endpoints (AuthorizationEndpoint, TokenEndpoint, etc.), those values will + take precedence over auto-discovery. This is useful for OAuth2 providers like GitHub that don't support OIDC discovery. + For CILogon, it's https://cilogon.org For Globus, it's https://auth.globus.org + For GitHub OAuth2, set this to https://github.com and explicitly configure the individual endpoints type: url default: https://cilogon.org components: ["registry", "origin", "cache", "director"] diff --git a/oa4mp/serve.go b/oa4mp/serve.go index 36899bd85..10251c350 100644 --- a/oa4mp/serve.go +++ b/oa4mp/serve.go @@ -233,7 +233,11 @@ func ConfigureOA4MP() (launcher daemon.Launcher, err error) { oidcAuthnUserClaim := param.Issuer_OIDCAuthenticationUserClaim.GetString() groupSource := param.Issuer_GroupSource.GetString() - if groupSource != "" && groupSource != "none" && groupSource != web_ui.GroupSourceTypeOIDC && groupSource != web_ui.GroupSourceTypeFile && groupSource != web_ui.GroupSourceTypeInternal { + if groupSource != "" && groupSource != "none" && + groupSource != web_ui.GroupSourceTypeOIDC && + groupSource != web_ui.GroupSourceTypeGitHub && + groupSource != web_ui.GroupSourceTypeFile && + groupSource != web_ui.GroupSourceTypeInternal { err = errors.New("invalid group source: " + groupSource) return } diff --git a/param/parameters.go b/param/parameters.go index 8abe0d1f1..efb6ff0c5 100644 --- a/param/parameters.go +++ b/param/parameters.go @@ -184,7 +184,9 @@ var runtimeConfigurableMap = map[string]bool{ "Issuer.OIDCAuthenticationRequirements": false, "Issuer.OIDCAuthenticationUserClaim": false, "Issuer.OIDCGroupClaim": false, + "Issuer.OIDCIssuerClaim": false, "Issuer.OIDCPreferClaimsFromIDToken": false, + "Issuer.OIDCSubjectClaim": false, "Issuer.QDLLocation": false, "Issuer.RedirectUris": false, "Issuer.ScitokensServerLocation": false, @@ -504,6 +506,10 @@ func (sP StringParam) GetString() string { return config.Issuer.OIDCAuthenticationUserClaim case "Issuer.OIDCGroupClaim": return config.Issuer.OIDCGroupClaim + case "Issuer.OIDCIssuerClaim": + return config.Issuer.OIDCIssuerClaim + case "Issuer.OIDCSubjectClaim": + return config.Issuer.OIDCSubjectClaim case "Issuer.QDLLocation": return config.Issuer.QDLLocation case "Issuer.ScitokensServerLocation": @@ -1287,7 +1293,9 @@ var allParameterNames = []string{ "Issuer.OIDCAuthenticationRequirements", "Issuer.OIDCAuthenticationUserClaim", "Issuer.OIDCGroupClaim", + "Issuer.OIDCIssuerClaim", "Issuer.OIDCPreferClaimsFromIDToken", + "Issuer.OIDCSubjectClaim", "Issuer.QDLLocation", "Issuer.RedirectUris", "Issuer.ScitokensServerLocation", @@ -1555,6 +1563,8 @@ var ( Issuer_IssuerClaimValue = StringParam{"Issuer.IssuerClaimValue"} Issuer_OIDCAuthenticationUserClaim = StringParam{"Issuer.OIDCAuthenticationUserClaim"} Issuer_OIDCGroupClaim = StringParam{"Issuer.OIDCGroupClaim"} + Issuer_OIDCIssuerClaim = StringParam{"Issuer.OIDCIssuerClaim"} + Issuer_OIDCSubjectClaim = StringParam{"Issuer.OIDCSubjectClaim"} Issuer_QDLLocation = StringParam{"Issuer.QDLLocation"} Issuer_ScitokensServerLocation = StringParam{"Issuer.ScitokensServerLocation"} Issuer_TomcatLocation = StringParam{"Issuer.TomcatLocation"} diff --git a/param/parameters_struct.go b/param/parameters_struct.go index c0cad25c9..f3a064f20 100644 --- a/param/parameters_struct.go +++ b/param/parameters_struct.go @@ -141,7 +141,9 @@ type Config struct { OIDCAuthenticationRequirements interface{} `mapstructure:"oidcauthenticationrequirements" yaml:"OIDCAuthenticationRequirements"` OIDCAuthenticationUserClaim string `mapstructure:"oidcauthenticationuserclaim" yaml:"OIDCAuthenticationUserClaim"` OIDCGroupClaim string `mapstructure:"oidcgroupclaim" yaml:"OIDCGroupClaim"` + OIDCIssuerClaim string `mapstructure:"oidcissuerclaim" yaml:"OIDCIssuerClaim"` OIDCPreferClaimsFromIDToken bool `mapstructure:"oidcpreferclaimsfromidtoken" yaml:"OIDCPreferClaimsFromIDToken"` + OIDCSubjectClaim string `mapstructure:"oidcsubjectclaim" yaml:"OIDCSubjectClaim"` QDLLocation string `mapstructure:"qdllocation" yaml:"QDLLocation"` RedirectUris []string `mapstructure:"redirecturis" yaml:"RedirectUris"` ScitokensServerLocation string `mapstructure:"scitokensserverlocation" yaml:"ScitokensServerLocation"` @@ -524,7 +526,9 @@ type configWithType struct { OIDCAuthenticationRequirements struct { Type string; Value interface{} } OIDCAuthenticationUserClaim struct { Type string; Value string } OIDCGroupClaim struct { Type string; Value string } + OIDCIssuerClaim struct { Type string; Value string } OIDCPreferClaimsFromIDToken struct { Type string; Value bool } + OIDCSubjectClaim struct { Type string; Value string } QDLLocation struct { Type string; Value string } RedirectUris struct { Type string; Value []string } ScitokensServerLocation struct { Type string; Value string } diff --git a/web_ui/oauth2_client.go b/web_ui/oauth2_client.go index 8c6a09ae8..e8f2fb8aa 100644 --- a/web_ui/oauth2_client.go +++ b/web_ui/oauth2_client.go @@ -57,6 +57,7 @@ const ( GroupSourceTypeOIDC string = "oidc" GroupSourceTypeFile string = "file" GroupSourceTypeInternal string = "internal" + GroupSourceTypeGitHub string = "github" ) var ( @@ -172,6 +173,51 @@ func handleOAuthLogin(ctx *gin.Context) { ctx.Redirect(http.StatusTemporaryRedirect, redirectUrl) } +// Fetch GitHub organization memberships for the authenticated user +// Uses the OAuth access token to call GitHub's /user/orgs endpoint +func fetchGitHubOrganizations(accessToken string) ([]string, error) { + client := config.GetClient() + + req, err := http.NewRequest(http.MethodGet, "https://api.github.com/user/orgs", nil) + if err != nil { + return nil, errors.Wrap(err, "failed to create GitHub orgs request") + } + req.Header.Set("Authorization", "Bearer "+accessToken) + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + resp, err := client.Do(req) + if err != nil { + return nil, errors.Wrap(err, "failed to fetch GitHub organizations") + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, errors.Wrap(err, "failed to read GitHub orgs response") + } + + if resp.StatusCode != http.StatusOK { + log.Errorf("GitHub orgs API returned status %d with body: %s", resp.StatusCode, string(body)) + return nil, errors.Errorf("GitHub orgs API returned status %d", resp.StatusCode) + } + + var orgs []struct { + Login string `json:"login"` + } + if err := json.Unmarshal(body, &orgs); err != nil { + return nil, errors.Wrap(err, "failed to parse GitHub orgs response") + } + + groups := make([]string, 0, len(orgs)) + for _, org := range orgs { + groups = append(groups, org.Login) + } + + log.Debugf("Fetched %d GitHub organizations for user", len(groups)) + return groups, nil +} + // Given a user name, return the list of groups they belong to func generateGroupInfo(user string) (groups []string, err error) { groupFile := param.Issuer_GroupFile.GetString() @@ -194,7 +240,8 @@ func generateGroupInfo(user string) (groups []string, err error) { // Given the maps for the UserInfo and ID token JSON objects, generate // user/group information according to the current policy. -func generateUserGroupInfo(userInfo map[string]interface{}, idToken map[string]interface{}) (userRecord *database.User, groups []string, err error) { +// The accessToken parameter is optional and only used when GroupSource is "github" +func generateUserGroupInfo(userInfo map[string]interface{}, idToken map[string]interface{}, accessToken string) (userRecord *database.User, groups []string, err error) { claimsSource := maps.Clone(userInfo) if param.Issuer_OIDCPreferClaimsFromIDToken.GetBool() { maps.Copy(claimsSource, idToken) @@ -228,36 +275,90 @@ func generateUserGroupInfo(userInfo map[string]interface{}, idToken map[string]i } username := userIdentifier - subIface, ok := claimsSource["sub"] - if !ok { - log.Errorln("User info endpoint did not return a value for the sub claim") - err = errors.New("identity provider did not return a subject for logged-in user") - return - } - sub, ok := subIface.(string) - if !ok { - log.Errorln("User info endpoint did not return a string for the sub claim") - err = errors.New("identity provider did not return a subject for logged-in user") - return + // Get the subject (sub) claim - this uniquely identifies the user at the identity provider + // For OIDC, this is the standard "sub" claim. For OAuth2 providers like GitHub, we may need + // to use a different claim (e.g., "id" for GitHub) + subClaim := param.Issuer_OIDCSubjectClaim.GetString() + if subClaim == "" { + subClaim = "sub" } - issuerClaim := param.Issuer_IssuerClaimValue.GetString() - if issuerClaim == "" { - issuerClaim = "iss" + subIface, ok := claimsSource[subClaim] + var sub string + if !ok { + // If the sub claim is not found, log a warning and fall back to the username + // This allows OAuth2 providers that don't provide a sub claim to still work + log.Warnf("User info endpoint did not return a value for the subject claim '%s', falling back to username '%s'", subClaim, username) + sub = username + } else { + // Try to convert the sub claim to a string + if subStr, ok := subIface.(string); ok { + sub = subStr + } else if subNum, ok := subIface.(float64); ok { + // Some providers (like GitHub) return a numeric ID + // Convert to int64 first to avoid floating-point precision issues + if subNum >= 0 && subNum <= float64(^uint64(0)>>1) && subNum == float64(int64(subNum)) { + sub = fmt.Sprintf("%d", int64(subNum)) + } else { + log.Errorf("User info endpoint returned an out-of-range numeric value for the subject claim '%s': %v", subClaim, subNum) + err = errors.New("identity provider returned an invalid numeric subject for logged-in user") + return + } + } else { + log.Errorf("User info endpoint returned a non-string/non-numeric value for the subject claim '%s'", subClaim) + err = errors.New("identity provider did not return a valid subject for logged-in user") + return + } } - issuerClaimValueIface, ok := claimsSource[issuerClaim] - if !ok { - log.Errorf("'%s' field of user info response from auth provider is not found", issuerClaim) - err = errors.New("identity provider returned an invalid issuer claim value") - return + // Get the issuer claim - this identifies the authentication provider + // For OIDC, this is the standard "iss" claim. For OAuth2 providers like GitHub, + // we may need to fall back to a configured value or the OIDC.Issuer setting + issuerClaimName := param.Issuer_OIDCIssuerClaim.GetString() + if issuerClaimName == "" { + issuerClaimName = "iss" } - issuerClaimValue, ok := issuerClaimValueIface.(string) + issuerClaimValueIface, ok := claimsSource[issuerClaimName] + var issuerClaimValue string if !ok { - log.Errorf("'%s' field of user info response from auth provider is not a string", issuerClaim) - err = errors.New("identity provider returned an invalid issuer claim value") - return + // If the issuer claim is not found in the user info, fall back to OIDC.Issuer or authorization endpoint + log.Warnf("User info endpoint did not return a value for the issuer claim '%s'", issuerClaimName) + + // Try to get from OIDC.Issuer configuration + if param.OIDC_Issuer.IsSet() { + issuerClaimValue = param.OIDC_Issuer.GetString() + log.Debugf("Using OIDC.Issuer as issuer: %s", issuerClaimValue) + } else if param.OIDC_AuthorizationEndpoint.IsSet() { + // Fall back to using the authorization endpoint hostname as the issuer + authEndpoint := param.OIDC_AuthorizationEndpoint.GetString() + parsedURL, parseErr := url.Parse(authEndpoint) + if parseErr == nil { + // Construct the issuer URL from scheme and host + issuerURL := &url.URL{ + Scheme: parsedURL.Scheme, + Host: parsedURL.Host, + } + issuerClaimValue = issuerURL.String() + log.Debugf("Using authorization endpoint host as issuer: %s", issuerClaimValue) + } else { + log.Errorf("Failed to parse authorization endpoint to determine issuer") + err = errors.New("identity provider did not return an issuer and unable to determine one from configuration") + return + } + } else { + log.Errorf("Unable to determine issuer: no '%s' claim in user info and OIDC.Issuer is not set", issuerClaimName) + err = errors.New("identity provider did not return an issuer claim value") + return + } + } else { + var ok bool + issuerClaimValue, ok = issuerClaimValueIface.(string) + if !ok { + log.Errorf("'%s' field of user info response from auth provider is not a string", issuerClaimName) + err = errors.New("identity provider returned an invalid issuer claim value") + return + } } // now that we have verified that the user belongs to a group we should create the user if it doesn't exist @@ -305,7 +406,17 @@ func generateUserGroupInfo(userInfo map[string]interface{}, idToken map[string]i for _, group := range groupList { groups = append(groups, group.Name) } - + case GroupSourceTypeGitHub: + if accessToken == "" { + log.Errorf("GitHub group source requires an access token") + err = errors.New("GitHub group source requires an access token") + return nil, nil, err + } + groups, err = fetchGitHubOrganizations(accessToken) + if err != nil { + log.Errorf("Failed to fetch GitHub organizations: %v", err) + return nil, nil, errors.Wrap(err, "failed to fetch GitHub organizations") + } case "", "none": log.Debugf("No group source specified; no groups will be used") return @@ -388,30 +499,37 @@ func handleOAuthCallback(ctx *gin.Context) { var idToken = make(map[string]interface{}) if idTokenRaw := token.Extra("id_token"); idTokenRaw != nil { - // We were given this ID token by the authentication provider, not - // some third party. If we don't trust the provider, we have greater - // issues. - skew, _ := time.ParseDuration("6s") - idTokenJWT, err := jwt.ParseString(idTokenRaw.(string), jwt.WithVerify(false), jwt.WithAcceptableSkew(skew)) - if err != nil { - log.Errorf("Error parsing OIDC ID token: %v", err) - ctx.JSON(http.StatusInternalServerError, - server_structs.SimpleApiResp{ - Status: server_structs.RespFailed, - Msg: fmt.Sprint("Error parsing OIDC ID token: ", ctx.Request.URL), - }) - return - } + // Check if the id_token is a non-empty string before parsing + // Some OAuth2 providers (e.g., GitHub) don't provide an ID token + idTokenStr, ok := idTokenRaw.(string) + if !ok || idTokenStr == "" { + log.Debugf("ID token is not a valid string or is empty") + } else { + // We were given this ID token by the authentication provider, not + // some third party. If we don't trust the provider, we have greater + // issues. + skew, _ := time.ParseDuration("6s") + idTokenJWT, err := jwt.ParseString(idTokenStr, jwt.WithVerify(false), jwt.WithAcceptableSkew(skew)) + if err != nil { + log.Errorf("Error parsing OIDC ID token: %v", err) + ctx.JSON(http.StatusInternalServerError, + server_structs.SimpleApiResp{ + Status: server_structs.RespFailed, + Msg: fmt.Sprint("Error parsing OIDC ID token: ", ctx.Request.URL), + }) + return + } - idToken, err = idTokenJWT.AsMap(ctx) - if err != nil { - log.Errorf("Error converting OIDC ID token to a map: %v", err) - ctx.JSON(http.StatusInternalServerError, - server_structs.SimpleApiResp{ - Status: server_structs.RespFailed, - Msg: fmt.Sprint("Error converting OIDC ID token to a map: ", ctx.Request.URL), - }) - return + idToken, err = idTokenJWT.AsMap(ctx) + if err != nil { + log.Errorf("Error converting OIDC ID token to a map: %v", err) + ctx.JSON(http.StatusInternalServerError, + server_structs.SimpleApiResp{ + Status: server_structs.RespFailed, + Msg: fmt.Sprint("Error converting OIDC ID token to a map: ", ctx.Request.URL), + }) + return + } } } else { log.Debugf("Did not find an OIDC ID token") @@ -477,7 +595,7 @@ func handleOAuthCallback(ctx *gin.Context) { return } - userRecord, groups, err := generateUserGroupInfo(userInfo, idToken) + userRecord, groups, err := generateUserGroupInfo(userInfo, idToken, token.AccessToken) if err != nil { ctx.JSON(http.StatusInternalServerError, server_structs.SimpleApiResp{