Add more logging and better error messages for openid authentication + clarify docs
continuous-integration/drone/push Build is passing Details

This commit is contained in:
kolaente 2021-05-16 13:23:10 +02:00
parent d695681a0e
commit b76ad8efe2
Signed by: konrad
GPG Key ID: F40E70337AB24C9B
4 changed files with 52 additions and 11 deletions

View File

@ -262,10 +262,12 @@ auth:
enabled: true enabled: true
# OpenID configuration will allow users to authenticate through a third-party OpenID Connect compatible provider.<br/> # OpenID configuration will allow users to authenticate through a third-party OpenID Connect compatible provider.<br/>
# The provider needs to support the `openid`, `profile` and `email` scopes.<br/> # The provider needs to support the `openid`, `profile` and `email` scopes.<br/>
# **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 <frontend-url>/auth/openid/<auth key>. Please make sure to configure the redirect url with your third party # to <frontend-url>/auth/openid/<auth key>. Please make sure to configure the redirect url with your third party
# auth service accordingy if you're using the default vikunja frontend. # 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: openid:
# Enable or disable OpenID Connect authentication # Enable or disable OpenID Connect authentication
enabled: false enabled: false

View File

@ -615,10 +615,12 @@ Default: `<empty>`
OpenID configuration will allow users to authenticate through a third-party OpenID Connect compatible provider.<br/> OpenID configuration will allow users to authenticate through a third-party OpenID Connect compatible provider.<br/>
The provider needs to support the `openid`, `profile` and `email` scopes.<br/> The provider needs to support the `openid`, `profile` and `email` scopes.<br/>
**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 <frontend-url>/auth/openid/<auth key>. Please make sure to configure the redirect url with your third party to <frontend-url>/auth/openid/<auth key>. Please make sure to configure the redirect url with your third party
auth service accordingy if you're using the default vikunja frontend. 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: `<empty>` Default: `<empty>`

View File

@ -23,6 +23,8 @@ import (
"net/http" "net/http"
"time" "time"
"code.vikunja.io/web/handler"
"code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/db"
"xorm.io/xorm" "xorm.io/xorm"
@ -86,7 +88,7 @@ func HandleCallback(c echo.Context) error {
provider, err := GetProvider(providerKey) provider, err := GetProvider(providerKey)
if err != nil { if err != nil {
log.Error(err) log.Error(err)
return err return handler.HandleHTTPError(err, c)
} }
if provider == nil { if provider == nil {
return c.JSON(http.StatusBadRequest, models.Message{Message: "Provider does not exist"}) 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{}) details := make(map[string]interface{})
if err := json.Unmarshal(rerr.Body, &details); err != nil { 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{}{ 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. // Extract the ID Token from OAuth2 token.
@ -123,14 +126,21 @@ func HandleCallback(c echo.Context) error {
// Parse and verify ID Token payload. // Parse and verify ID Token payload.
idToken, err := verifier.Verify(context.Background(), rawIDToken) idToken, err := verifier.Verify(context.Background(), rawIDToken)
if err != nil { 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 // Extract custom claims
cl := &claims{} cl := &claims{}
err = idToken.Claims(cl) err = idToken.Claims(cl)
if err != nil { 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() s := db.NewSession()
@ -140,12 +150,13 @@ func HandleCallback(c echo.Context) error {
u, err := getOrCreateUser(s, cl, idToken.Issuer, idToken.Subject) u, err := getOrCreateUser(s, cl, idToken.Issuer, idToken.Subject)
if err != nil { if err != nil {
_ = s.Rollback() _ = 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() err = s.Commit()
if err != nil { if err != nil {
return err return handler.HandleHTTPError(err, c)
} }
// Create token // Create token

View File

@ -399,3 +399,29 @@ func (err ErrInvalidAvatarProvider) HTTPError() web.HTTPError {
Message: "Invalid avatar provider setting. See docs for valid types.", 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.",
}
}