From b76ad8efe27d2817452b4b525b3459962b5c7de9 Mon Sep 17 00:00:00 2001 From: kolaente Date: Sun, 16 May 2021 13:23:10 +0200 Subject: [PATCH] Add more logging and better error messages for openid authentication + clarify docs --- config.yml.sample | 6 ++++-- docs/content/doc/setup/config.md | 6 ++++-- pkg/modules/auth/openid/openid.go | 25 ++++++++++++++++++------- pkg/user/error.go | 26 ++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/config.yml.sample b/config.yml.sample index 36407d049..f185d1c10 100644 --- a/config.yml.sample +++ b/config.yml.sample @@ -262,10 +262,12 @@ auth: enabled: true # OpenID configuration will allow users to authenticate through a third-party OpenID Connect compatible provider.
# The provider needs to support the `openid`, `profile` and `email` scopes.
- # **Note:** The frontend expects to be redirected after authentication by the third party + # **Note:** Some openid providers (like gitlab) only make the email of the user available through openid claims if they have set it to be publicly visible. + # If the email is not public in those cases, authenticating will fail. + # **Note 2:** The frontend expects to be redirected after authentication by the third party # to /auth/openid/. Please make sure to configure the redirect url with your third party # auth service accordingy if you're using the default vikunja frontend. - # Take a look at the [default config file](https://kolaente.dev/vikunja/api/src/branch/master/config.yml.sample) for more information about how to configure openid authentication. + # Take a look at the [default config file](https://kolaente.dev/vikunja/api/src/branch/main/config.yml.sample) for more information about how to configure openid authentication. openid: # Enable or disable OpenID Connect authentication enabled: false diff --git a/docs/content/doc/setup/config.md b/docs/content/doc/setup/config.md index c9e3db33d..8ab8a954f 100644 --- a/docs/content/doc/setup/config.md +++ b/docs/content/doc/setup/config.md @@ -615,10 +615,12 @@ Default: `` OpenID configuration will allow users to authenticate through a third-party OpenID Connect compatible provider.
The provider needs to support the `openid`, `profile` and `email` scopes.
-**Note:** The frontend expects to be redirected after authentication by the third party +**Note:** Some openid providers (like gitlab) only make the email of the user available through openid claims if they have set it to be publicly visible. +If the email is not public in those cases, authenticating will fail. +**Note 2:** The frontend expects to be redirected after authentication by the third party to /auth/openid/. Please make sure to configure the redirect url with your third party auth service accordingy if you're using the default vikunja frontend. -Take a look at the [default config file](https://kolaente.dev/vikunja/api/src/branch/master/config.yml.sample) for more information about how to configure openid authentication. +Take a look at the [default config file](https://kolaente.dev/vikunja/api/src/branch/main/config.yml.sample) for more information about how to configure openid authentication. Default: `` diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index baa3b42dd..8cd169bd3 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -23,6 +23,8 @@ import ( "net/http" "time" + "code.vikunja.io/web/handler" + "code.vikunja.io/api/pkg/db" "xorm.io/xorm" @@ -86,7 +88,7 @@ func HandleCallback(c echo.Context) error { provider, err := GetProvider(providerKey) if err != nil { log.Error(err) - return err + return handler.HandleHTTPError(err, c) } if provider == nil { return c.JSON(http.StatusBadRequest, models.Message{Message: "Provider does not exist"}) @@ -100,7 +102,8 @@ func HandleCallback(c echo.Context) error { details := make(map[string]interface{}) if err := json.Unmarshal(rerr.Body, &details); err != nil { - return err + log.Errorf("Error unmarshaling token for provider %s: %v", provider.Name, err) + return handler.HandleHTTPError(err, c) } return c.JSON(http.StatusBadRequest, map[string]interface{}{ @@ -109,7 +112,7 @@ func HandleCallback(c echo.Context) error { }) } - return err + return handler.HandleHTTPError(err, c) } // Extract the ID Token from OAuth2 token. @@ -123,14 +126,21 @@ func HandleCallback(c echo.Context) error { // Parse and verify ID Token payload. idToken, err := verifier.Verify(context.Background(), rawIDToken) if err != nil { - return err + log.Errorf("Error verifying token for provider %s: %v", provider.Name, err) + return handler.HandleHTTPError(err, c) } // Extract custom claims cl := &claims{} err = idToken.Claims(cl) if err != nil { - return err + log.Errorf("Error getting token claims for provider %s: %v", provider.Name, err) + return handler.HandleHTTPError(err, c) + } + + if cl.Email == "" { + log.Errorf("Claim does not contain an email address for provider %s", provider.Name) + return handler.HandleHTTPError(&user.ErrNoOpenIDEmailProvided{}, c) } s := db.NewSession() @@ -140,12 +150,13 @@ func HandleCallback(c echo.Context) error { u, err := getOrCreateUser(s, cl, idToken.Issuer, idToken.Subject) if err != nil { _ = s.Rollback() - return err + log.Errorf("Error creating new user for provider %s: %v", provider.Name, err) + return handler.HandleHTTPError(err, c) } err = s.Commit() if err != nil { - return err + return handler.HandleHTTPError(err, c) } // Create token diff --git a/pkg/user/error.go b/pkg/user/error.go index 0328cfb86..6480d5ffe 100644 --- a/pkg/user/error.go +++ b/pkg/user/error.go @@ -399,3 +399,29 @@ func (err ErrInvalidAvatarProvider) HTTPError() web.HTTPError { Message: "Invalid avatar provider setting. See docs for valid types.", } } + +// ErrNoOpenIDEmailProvided represents a "NoEmailProvided" kind of error. +type ErrNoOpenIDEmailProvided struct { +} + +// IsErrNoEmailProvided checks if an error is a ErrNoOpenIDEmailProvided. +func IsErrNoEmailProvided(err error) bool { + _, ok := err.(*ErrNoOpenIDEmailProvided) + return ok +} + +func (err *ErrNoOpenIDEmailProvided) Error() string { + return "No email provided" +} + +// ErrCodeNoOpenIDEmailProvided holds the unique world-error code of this error +const ErrCodeNoOpenIDEmailProvided = 1019 + +// HTTPError holds the http error description +func (err *ErrNoOpenIDEmailProvided) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusPreconditionFailed, + Code: ErrCodeNoOpenIDEmailProvided, + Message: "No email address available. Please make sure the openid provider publicly provides an email address for your account.", + } +}