From fbce915d8487708ba5e65208c4db700077abc8c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Fri, 8 Sep 2023 19:52:28 -0700 Subject: [PATCH] Add profile scope to OIDC integration to support accounts without email --- internal/config/config_test.go | 10 +++++----- internal/config/options.go | 12 ++++++------ internal/config/parser.go | 2 +- internal/oauth2/oidc.go | 35 ++++++++++++++++++++++++++++++---- internal/ui/oauth2.go | 2 +- 5 files changed, 44 insertions(+), 17 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d10c4471..18bfe4ce 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -917,7 +917,7 @@ func TestDefaultOAuth2RedirectURLValue(t *testing.T) { } } -func TestOAuth2OidcDiscoveryEndpoint(t *testing.T) { +func TestOAuth2OIDCDiscoveryEndpoint(t *testing.T) { os.Clearenv() os.Setenv("OAUTH2_OIDC_DISCOVERY_ENDPOINT", "http://example.org") @@ -928,14 +928,14 @@ func TestOAuth2OidcDiscoveryEndpoint(t *testing.T) { } expected := "http://example.org" - result := opts.OAuth2OidcDiscoveryEndpoint() + result := opts.OIDCDiscoveryEndpoint() if result != expected { t.Fatalf(`Unexpected OAUTH2_OIDC_DISCOVERY_ENDPOINT value, got %q instead of %q`, result, expected) } } -func TestDefaultOAuth2OidcDiscoveryEndpointValue(t *testing.T) { +func TestDefaultOIDCDiscoveryEndpointValue(t *testing.T) { os.Clearenv() parser := NewParser() @@ -945,10 +945,10 @@ func TestDefaultOAuth2OidcDiscoveryEndpointValue(t *testing.T) { } expected := defaultOAuth2OidcDiscoveryEndpoint - result := opts.OAuth2OidcDiscoveryEndpoint() + result := opts.OIDCDiscoveryEndpoint() if result != expected { - t.Fatalf(`Unexpected OAUTH2_REDIRECT_URL value, got %q instead of %q`, result, expected) + t.Fatalf(`Unexpected OAUTH2_OIDC_DISCOVERY_ENDPOINT value, got %q instead of %q`, result, expected) } } diff --git a/internal/config/options.go b/internal/config/options.go index d4a56d4b..16d16cbd 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -136,7 +136,7 @@ type Options struct { oauth2ClientID string oauth2ClientSecret string oauth2RedirectURL string - oauth2OidcDiscoveryEndpoint string + oidcDiscoveryEndpoint string oauth2Provider string pocketConsumerKey string httpClientTimeout int @@ -208,7 +208,7 @@ func NewOptions() *Options { oauth2ClientID: defaultOAuth2ClientID, oauth2ClientSecret: defaultOAuth2ClientSecret, oauth2RedirectURL: defaultOAuth2RedirectURL, - oauth2OidcDiscoveryEndpoint: defaultOAuth2OidcDiscoveryEndpoint, + oidcDiscoveryEndpoint: defaultOAuth2OidcDiscoveryEndpoint, oauth2Provider: defaultOAuth2Provider, pocketConsumerKey: defaultPocketConsumerKey, httpClientTimeout: defaultHTTPClientTimeout, @@ -401,9 +401,9 @@ func (o *Options) OAuth2RedirectURL() string { return o.oauth2RedirectURL } -// OAuth2OidcDiscoveryEndpoint returns the OAuth2 OIDC discovery endpoint. -func (o *Options) OAuth2OidcDiscoveryEndpoint() string { - return o.oauth2OidcDiscoveryEndpoint +// OIDCDiscoveryEndpoint returns the OAuth2 OIDC discovery endpoint. +func (o *Options) OIDCDiscoveryEndpoint() string { + return o.oidcDiscoveryEndpoint } // OAuth2Provider returns the name of the OAuth2 provider configured. @@ -619,7 +619,7 @@ func (o *Options) SortedOptions(redactSecret bool) []*Option { "METRICS_USERNAME": o.metricsUsername, "OAUTH2_CLIENT_ID": o.oauth2ClientID, "OAUTH2_CLIENT_SECRET": redactSecretValue(o.oauth2ClientSecret, redactSecret), - "OAUTH2_OIDC_DISCOVERY_ENDPOINT": o.oauth2OidcDiscoveryEndpoint, + "OAUTH2_OIDC_DISCOVERY_ENDPOINT": o.oidcDiscoveryEndpoint, "OAUTH2_PROVIDER": o.oauth2Provider, "OAUTH2_REDIRECT_URL": o.oauth2RedirectURL, "OAUTH2_USER_CREATION": o.oauth2UserCreationAllowed, diff --git a/internal/config/parser.go b/internal/config/parser.go index bb43bafa..d0d19af3 100644 --- a/internal/config/parser.go +++ b/internal/config/parser.go @@ -180,7 +180,7 @@ func (p *Parser) parseLines(lines []string) (err error) { case "OAUTH2_REDIRECT_URL": p.opts.oauth2RedirectURL = parseString(value, defaultOAuth2RedirectURL) case "OAUTH2_OIDC_DISCOVERY_ENDPOINT": - p.opts.oauth2OidcDiscoveryEndpoint = parseString(value, defaultOAuth2OidcDiscoveryEndpoint) + p.opts.oidcDiscoveryEndpoint = parseString(value, defaultOAuth2OidcDiscoveryEndpoint) case "OAUTH2_PROVIDER": p.opts.oauth2Provider = parseString(value, defaultOAuth2Provider) case "HTTP_CLIENT_TIMEOUT": diff --git a/internal/oauth2/oidc.go b/internal/oauth2/oidc.go index 8fcb5527..6d9784f1 100644 --- a/internal/oauth2/oidc.go +++ b/internal/oauth2/oidc.go @@ -28,10 +28,15 @@ type oidcProvider struct { func NewOidcProvider(ctx context.Context, clientID, clientSecret, redirectURL, discoveryEndpoint string) (*oidcProvider, error) { provider, err := oidc.NewProvider(ctx, discoveryEndpoint) if err != nil { - return nil, err + return nil, fmt.Errorf(`oidc: failed to initialize provider %q: %w`, discoveryEndpoint, err) } - return &oidcProvider{clientID: clientID, clientSecret: clientSecret, redirectURL: redirectURL, provider: provider}, nil + return &oidcProvider{ + clientID: clientID, + clientSecret: clientSecret, + redirectURL: redirectURL, + provider: provider, + }, nil } func (o *oidcProvider) GetUserExtraKey() string { @@ -43,7 +48,7 @@ func (o *oidcProvider) GetConfig() *oauth2.Config { RedirectURL: o.redirectURL, ClientID: o.clientID, ClientSecret: o.clientSecret, - Scopes: []string{"openid", "email"}, + Scopes: []string{oidc.ScopeOpenID, "profile", "email"}, Endpoint: o.provider.Endpoint(), } } @@ -60,7 +65,22 @@ func (o *oidcProvider) GetProfile(ctx context.Context, code, codeVerifier string return nil, fmt.Errorf(`oidc: failed to get user info: %w`, err) } - profile := &Profile{Key: o.GetUserExtraKey(), ID: userInfo.Subject, Username: userInfo.Email} + profile := &Profile{ + Key: o.GetUserExtraKey(), + ID: userInfo.Subject, + } + + var userClaims userClaims + if err := userInfo.Claims(&userClaims); err != nil { + return nil, fmt.Errorf(`oidc: failed to parse user claims: %w`, err) + } + + for _, value := range []string{userClaims.Email, userClaims.PreferredUsername, userClaims.Name, userClaims.Profile} { + if value != "" { + profile.Username = value + break + } + } if profile.Username == "" { return nil, ErrEmptyUsername @@ -80,3 +100,10 @@ func (o *oidcProvider) PopulateUserWithProfileID(user *model.User, profile *Prof func (o *oidcProvider) UnsetUserProfileID(user *model.User) { user.OpenIDConnectID = "" } + +type userClaims struct { + Email string `json:"email"` + Profile string `json:"profile"` + Name string `json:"name"` + PreferredUsername string `json:"preferred_username"` +} diff --git a/internal/ui/oauth2.go b/internal/ui/oauth2.go index 72b360d5..b6547f95 100644 --- a/internal/ui/oauth2.go +++ b/internal/ui/oauth2.go @@ -16,6 +16,6 @@ func getOAuth2Manager(ctx context.Context) *oauth2.Manager { config.Opts.OAuth2ClientID(), config.Opts.OAuth2ClientSecret(), config.Opts.OAuth2RedirectURL(), - config.Opts.OAuth2OidcDiscoveryEndpoint(), + config.Opts.OIDCDiscoveryEndpoint(), ) }