From 2fa6523cc1ad67745699a00919ba83be1e70986c Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Wed, 25 Nov 2020 21:25:57 -0500 Subject: [PATCH 1/9] Add support to login using identity from an identity-aware proxy --- config.yml.sample | 13 +- docs/content/doc/setup/config.md | 9 + go.mod | 1 + go.sum | 8 + pkg/config/config.go | 12 +- pkg/integrations/integrations.go | 12 +- pkg/models/link_sharing.go | 12 - pkg/modules/auth/auth.go | 286 ++++++++++++++++-- .../{openid/openid_test.go => auth_test.go} | 31 +- pkg/modules/auth/identityawareproxy/error.go | 135 +++++++++ .../identityawareproxy/identityawareproxy.go | 165 ++++++++++ .../identityawareproxy_test.go | 19 ++ .../auth/identityawareproxy/middleware.go | 132 ++++++++ .../identityawareproxy/middleware_test.go | 130 ++++++++ pkg/modules/auth/{openid => }/main_test.go | 2 +- pkg/modules/auth/openid/openid.go | 76 +---- pkg/modules/migration/handler/handler.go | 6 +- pkg/routes/api/v1/avatar.go | 3 +- pkg/routes/api/v1/info.go | 12 +- pkg/routes/api/v1/list_by_namespace.go | 6 +- pkg/routes/api/v1/login.go | 34 +-- pkg/routes/api/v1/token_check.go | 7 +- pkg/routes/api/v1/user_settings.go | 7 +- pkg/routes/api/v1/user_totp.go | 11 +- pkg/routes/api/v1/user_update_email.go | 3 +- pkg/routes/api/v1/user_update_password.go | 3 +- pkg/routes/routes.go | 22 +- pkg/swagger/docs.go | 48 +++ pkg/swagger/swagger.json | 48 +++ pkg/swagger/swagger.yaml | 30 ++ pkg/user/user.go | 25 -- 31 files changed, 1069 insertions(+), 239 deletions(-) rename pkg/modules/auth/{openid/openid_test.go => auth_test.go} (74%) create mode 100644 pkg/modules/auth/identityawareproxy/error.go create mode 100644 pkg/modules/auth/identityawareproxy/identityawareproxy.go create mode 100644 pkg/modules/auth/identityawareproxy/identityawareproxy_test.go create mode 100644 pkg/modules/auth/identityawareproxy/middleware.go create mode 100644 pkg/modules/auth/identityawareproxy/middleware_test.go rename pkg/modules/auth/{openid => }/main_test.go (98%) diff --git a/config.yml.sample b/config.yml.sample index f185d1c10..33bcd1a11 100644 --- a/config.yml.sample +++ b/config.yml.sample @@ -285,6 +285,17 @@ auth: # The client secret used to authenticate Vikunja at the OpenID Connect provider. clientsecret: + # Identity-Aware Proxy configuration will allow users to authenticate through an identity-aware proxy in front of Vikunja.
+ # The proxy needs to supply a signed JWT over HTTP headers with claims for `sub`, `iss`, and `email`. + # It must also serve the public keys for that signed JWT in the JWKS format. + identityawareproxy: + # Enable or disable Identity-Aware Proxy authentication + enabled: false + # The URL of the proxy's JWKS endpoint + jwksuri: + # The HTTP header containing the signed JWT from the proxy + jwtheader: + # Prometheus metrics endpoint metrics: # If set to true, enables a /metrics endpoint for prometheus to collect metrics about Vikunja. @@ -292,4 +303,4 @@ metrics: # If set to a non-empty value the /metrics endpoint will require this as a username via basic auth in combination with the password below. username: # If set to a non-empty value the /metrics endpoint will require this as a password via basic auth in combination with the username below. - password: + password: \ No newline at end of file diff --git a/docs/content/doc/setup/config.md b/docs/content/doc/setup/config.md index ed08631f1..d4fdaedb5 100644 --- a/docs/content/doc/setup/config.md +++ b/docs/content/doc/setup/config.md @@ -650,3 +650,12 @@ If set to a non-empty value the /metrics endpoint will require this as a passwor Default: `` +### identityawareproxy + +Identity-Aware Proxy configuration will allow users to authenticate through an identity-aware proxy in front of Vikunja.
+The proxy needs to supply a signed JWT over HTTP headers with claims for `sub`, `iss`, and `email`. +This JWT must be signed using ECDSA or RSA. +It must also serve the public keys for that signed JWT in the JWKS format. +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 this authentication. + +Default: `` \ No newline at end of file diff --git a/go.mod b/go.mod index 3bb46f176..096d4e953 100644 --- a/go.mod +++ b/go.mod @@ -45,6 +45,7 @@ require ( github.com/labstack/echo/v4 v4.3.0 github.com/labstack/gommon v0.3.0 github.com/laurent22/ical-go v0.1.1-0.20181107184520-7e5d6ade8eef + github.com/lestrrat-go/jwx v1.0.6 github.com/lib/pq v1.10.2 github.com/magefile/mage v1.11.0 github.com/mailru/easyjson v0.7.6 // indirect diff --git a/go.sum b/go.sum index 310794c1f..abbbfdbcf 100644 --- a/go.sum +++ b/go.sum @@ -466,6 +466,13 @@ github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL github.com/laurent22/ical-go v0.1.1-0.20181107184520-7e5d6ade8eef h1:RZnRnSID1skF35j/15KJ6hKZkdIC/teQClJK5wP5LU4= github.com/laurent22/ical-go v0.1.1-0.20181107184520-7e5d6ade8eef/go.mod h1:4LATl0uhhtytR6p9n1AlktDyIz4u2iUnWEdI3L/hXiw= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= +github.com/lestrrat-go/iter v0.0.0-20200422075355-fc1769541911 h1:FvnrqecqX4zT0wOIbYK1gNgTm0677INEWiFY8UEYggY= +github.com/lestrrat-go/iter v0.0.0-20200422075355-fc1769541911/go.mod h1:zIdgO1mRKhn8l9vrZJZz9TUMMFbQbLeTsbqPDrJ/OJc= +github.com/lestrrat-go/jwx v1.0.6 h1:0absmJ/XlsxNkXr9syeIHjCJnu3rZa+DKzdCI6QfYgU= +github.com/lestrrat-go/jwx v1.0.6/go.mod h1:NNxs6i86gQDGEqgIszN/pkJihMqzYrXMIJt2Yhxhkvs= +github.com/lestrrat-go/pdebug v0.0.0-20200204225717-4d6bd78da58d h1:aEZT3f1GGg5RIlHMAy4/4fe4ciOi3SCwYoaURphcB4k= +github.com/lestrrat-go/pdebug v0.0.0-20200204225717-4d6bd78da58d/go.mod h1:B06CSso/AWxiPejj+fheUINGeBKeeEZNt8w+EoU7+L8= +github.com/lib/pq v1.0.0 h1:X5PMW56eZitiTeO7tKzZxFCSpbFZJtkMMooicw2us9A= github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.1.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= @@ -1038,6 +1045,7 @@ golang.org/x/tools v0.0.0-20200227222343-706bc42d1f0d/go.mod h1:TB2adYChydJhpapK golang.org/x/tools v0.0.0-20200304193943-95d2e580d8eb/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw= golang.org/x/tools v0.0.0-20200312045724-11d5b4c81c7d/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw= golang.org/x/tools v0.0.0-20200331025713-a30bf2db82d4/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8= +golang.org/x/tools v0.0.0-20200417140056-c07e33ef3290/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200501065659-ab2804fb9c9d/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200512131952-2bc93b1c0c88/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200515010526-7d3b6ebf133d/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= diff --git a/pkg/config/config.go b/pkg/config/config.go index c10cc04c4..2dfd436c5 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -55,10 +55,13 @@ const ( ServiceTestingtoken Key = `service.testingtoken` ServiceEnableEmailReminders Key = `service.enableemailreminders` - AuthLocalEnabled Key = `auth.local.enabled` - AuthOpenIDEnabled Key = `auth.openid.enabled` - AuthOpenIDRedirectURL Key = `auth.openid.redirecturl` - AuthOpenIDProviders Key = `auth.openid.providers` + AuthLocalEnabled Key = `auth.local.enabled` + AuthOpenIDEnabled Key = `auth.openid.enabled` + AuthOpenIDRedirectURL Key = `auth.openid.redirecturl` + AuthOpenIDProviders Key = `auth.openid.providers` + AuthIdentityAwareProxyEnabled Key = `auth.identityawareproxy.enabled` + AuthIdentityAwareProxyJwksUri Key = `auth.identityawareproxy.jwksuri` + AuthIdentityAwareProxyJwtHeader Key = `auth.identityawareproxy.jwtheader` LegalImprintURL Key = `legal.imprinturl` LegalPrivacyURL Key = `legal.privacyurl` @@ -247,6 +250,7 @@ func InitDefaultConfig() { // Auth AuthLocalEnabled.setDefault(true) AuthOpenIDEnabled.setDefault(false) + AuthIdentityAwareProxyEnabled.setDefault(false) // Database DatabaseType.setDefault("sqlite") diff --git a/pkg/integrations/integrations.go b/pkg/integrations/integrations.go index f9c8a69ea..f995732c6 100644 --- a/pkg/integrations/integrations.go +++ b/pkg/integrations/integrations.go @@ -121,27 +121,31 @@ func newTestRequest(t *testing.T, method string, handler func(ctx echo.Context) } func addUserTokenToContext(t *testing.T, user *user.User, c echo.Context) { + jwtConfig := auth.GetJWTConfig() + // Get the token as a string token, err := auth.NewUserJWTAuthtoken(user) assert.NoError(t, err) // We send the string token through the parsing function to get a valid jwt.Token - tken, err := jwt.Parse(token, func(t *jwt.Token) (interface{}, error) { + tken, err := jwt.ParseWithClaims(token, jwtConfig.Claims, func(t *jwt.Token) (interface{}, error) { return []byte(config.ServiceJWTSecret.GetString()), nil }) assert.NoError(t, err) - c.Set("user", tken) + c.Set(jwtConfig.ContextKey, tken) } func addLinkShareTokenToContext(t *testing.T, share *models.LinkSharing, c echo.Context) { + jwtConfig := auth.GetJWTConfig() + // Get the token as a string token, err := auth.NewLinkShareJWTAuthtoken(share) assert.NoError(t, err) // We send the string token through the parsing function to get a valid jwt.Token - tken, err := jwt.Parse(token, func(t *jwt.Token) (interface{}, error) { + tken, err := jwt.ParseWithClaims(token, jwtConfig.Claims, func(t *jwt.Token) (interface{}, error) { return []byte(config.ServiceJWTSecret.GetString()), nil }) assert.NoError(t, err) - c.Set("user", tken) + c.Set(jwtConfig.ContextKey, tken) } func testRequestSetup(t *testing.T, method string, payload string, queryParams url.Values, urlParams map[string]string) (rec *httptest.ResponseRecorder, c echo.Context) { diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go index 73a531e91..7ee6c9eca 100644 --- a/pkg/models/link_sharing.go +++ b/pkg/models/link_sharing.go @@ -25,7 +25,6 @@ import ( "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/utils" "code.vikunja.io/web" - "github.com/dgrijalva/jwt-go" "xorm.io/xorm" ) @@ -81,17 +80,6 @@ func (share *LinkSharing) GetID() int64 { return share.ID } -// GetLinkShareFromClaims builds a link sharing object from jwt claims -func GetLinkShareFromClaims(claims jwt.MapClaims) (share *LinkSharing, err error) { - share = &LinkSharing{} - share.ID = int64(claims["id"].(float64)) - share.Hash = claims["hash"].(string) - share.ListID = int64(claims["list_id"].(float64)) - share.Right = Right(claims["right"].(float64)) - share.SharedByID = int64(claims["sharedByID"].(float64)) - return -} - func (share *LinkSharing) getUserID() int64 { return share.ID * -1 } diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index 167d44b2c..4f0d2aa44 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -20,26 +20,78 @@ import ( "net/http" "time" + "xorm.io/xorm" + "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" "github.com/dgrijalva/jwt-go" + petname "github.com/dustinkirkland/golang-petname" "github.com/labstack/echo/v4" + "github.com/labstack/echo/v4/middleware" ) // These are all valid auth types +type AuthType int + const ( - AuthTypeUnknown int = iota + AuthTypeUnknown AuthType = iota AuthTypeUser AuthTypeLinkShare + AuthTypeIAPUser ) -// Token represents an authentification token +const authTokenContextKey string = "authToken" + +// Token represents an authentification token in signed string form type Token struct { Token string `json:"token"` } +// Claims made in the JWT token for various auth types +// Only the auth module should introspect and handle claims +type AuthClaims struct { + // Common to all claims + Type AuthType `json:"type"` + + // AuthTypeUser and AuthTypeIAPUser claims + UserID int64 `json:"user_id,omitempty"` + UserUsername string `json:"user_username,omitempty"` + UserEmail string `json:"user_email,omitempty"` + UserName string `json:"user_name,omitempty"` + + UserEmailRemindersEnabled bool `json:"user_email_reminders_enabled"` + + // AuthTypeLinkShare claims + ShareID int64 `json:"share_id,omitempty"` + ShareHash string `json:"share_hash,omitempty"` + ShareListID int64 `json:"share_list_id,omitempty"` + ShareRight models.Right `json:"share_right,omitempty"` + ShareSharedByID int64 `json:"share_shared_by_id,omitempty"` + + // Common claims + jwt.StandardClaims +} + +// An AuthProvider provides alternative methods of authentication +// In these cases, AuthClaims may contain hints to the user identity, +// but an outside source is the final source-of-truth for auth (e.g. Identity-Aware Proxy auth) +type AuthProvider interface { + GetWebAuth(echo.Context, *AuthClaims) (web.Auth, error) +} + +var authProviders = map[AuthType]AuthProvider{} + +func RegisterAuthProvider(t AuthType, provider AuthProvider) { + authProviders[t] = provider +} + +// NewTokenResponse creates a new token response from a token +func NewTokenResponse(t string, c echo.Context) error { + return c.JSON(http.StatusOK, Token{Token: t}) +} + // NewUserAuthTokenResponse creates a new user auth token response from a user object. func NewUserAuthTokenResponse(u *user.User, c echo.Context) error { t, err := NewUserJWTAuthtoken(u) @@ -47,22 +99,25 @@ func NewUserAuthTokenResponse(u *user.User, c echo.Context) error { return err } - return c.JSON(http.StatusOK, Token{Token: t}) + return NewTokenResponse(t, c) } // NewUserJWTAuthtoken generates and signes a new jwt token for a user. This is a global function to be able to call it from integration tests. -func NewUserJWTAuthtoken(user *user.User) (token string, err error) { - t := jwt.New(jwt.SigningMethodHS256) - +func NewUserJWTAuthtoken(u *user.User) (token string, err error) { // Set claims - claims := t.Claims.(jwt.MapClaims) - claims["type"] = AuthTypeUser - claims["id"] = user.ID - claims["username"] = user.Username - claims["email"] = user.Email - claims["exp"] = time.Now().Add(time.Hour * 72).Unix() - claims["name"] = user.Name - claims["emailRemindersEnabled"] = user.EmailRemindersEnabled + claims := &AuthClaims{ + Type: AuthTypeUser, + UserID: u.ID, + UserUsername: u.Username, + UserEmail: u.Email, + UserName: u.Name, + UserEmailRemindersEnabled: u.EmailRemindersEnabled, + StandardClaims: jwt.StandardClaims{ + ExpiresAt: time.Now().Add(time.Hour * 72).Unix(), + }, + } + + t := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) // Generate encoded token and send it as response. return t.SignedString([]byte(config.ServiceJWTSecret.GetString())) @@ -70,32 +125,199 @@ func NewUserJWTAuthtoken(user *user.User) (token string, err error) { // NewLinkShareJWTAuthtoken creates a new jwt token from a link share func NewLinkShareJWTAuthtoken(share *models.LinkSharing) (token string, err error) { - t := jwt.New(jwt.SigningMethodHS256) - // Set claims - claims := t.Claims.(jwt.MapClaims) - claims["type"] = AuthTypeLinkShare - claims["id"] = share.ID - claims["hash"] = share.Hash - claims["list_id"] = share.ListID - claims["right"] = share.Right - claims["sharedByID"] = share.SharedByID - claims["exp"] = time.Now().Add(time.Hour * 72).Unix() + claims := &AuthClaims{ + Type: AuthTypeLinkShare, + ShareID: share.ID, + ShareHash: share.Hash, + ShareListID: share.ListID, + ShareRight: share.Right, + ShareSharedByID: share.SharedByID, + StandardClaims: jwt.StandardClaims{ + ExpiresAt: time.Now().Add(time.Hour * 72).Unix(), + }, + } + + t := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) // Generate encoded token and send it as response. return t.SignedString([]byte(config.ServiceJWTSecret.GetString())) } -// GetAuthFromClaims returns a web.Auth object from jwt claims +// GetAuthFromClaims returns a web.Auth object from jwt claims or from an +// alternative authProvider func GetAuthFromClaims(c echo.Context) (a web.Auth, err error) { - jwtinf := c.Get("user").(*jwt.Token) - claims := jwtinf.Claims.(jwt.MapClaims) - typ := int(claims["type"].(float64)) - if typ == AuthTypeLinkShare && config.ServiceEnableLinkSharing.GetBool() { - return models.GetLinkShareFromClaims(claims) + jwtinf := c.Get(authTokenContextKey).(*jwt.Token) + claims := jwtinf.Claims.(*AuthClaims) + if claims.Type == AuthTypeLinkShare && config.ServiceEnableLinkSharing.GetBool() { + return getLinkShareFromClaims(claims) } - if typ == AuthTypeUser { - return user.GetUserFromClaims(claims) + if claims.Type == AuthTypeUser { + return getUserFromClaims(claims) + } + if authProvider, ok := authProviders[claims.Type]; ok { + return authProvider.GetWebAuth(c, claims) } return nil, echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) } + +// getLinkShareFromClaims builds a link sharing object from jwt claims +func getLinkShareFromClaims(claims *AuthClaims) (share *models.LinkSharing, err error) { + share = &models.LinkSharing{} + share.ID = claims.ShareID + share.Hash = claims.ShareHash + share.ListID = claims.ShareListID + share.Right = claims.ShareRight + share.SharedByID = claims.ShareSharedByID + + if share.Hash == "" { + return nil, echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) + } + + return +} + +// getUserFromClaims Returns a new user from jwt claims +func getUserFromClaims(claims *AuthClaims) (u *user.User, err error) { + u = &user.User{ + ID: claims.UserID, + Email: claims.UserEmail, + Username: claims.UserUsername, + Name: claims.UserName, + } + + return +} + +// GetCurrentUser returns the current user based on its jwt token +func GetCurrentUser(c echo.Context) (u *user.User, err error) { + auth, err := GetAuthFromClaims(c) + if err != nil { + return nil, err + } + + u, ok := auth.(*user.User) + if !ok { + return nil, user.ErrCouldNotGetUserID{} + } + + return u, nil +} + +// Generates a new jwt token for the types AuthTypeLinkShare and AuthTypeUser +func RenewToken(s *xorm.Session, c echo.Context) (token string, err error) { + jwtinf := c.Get(authTokenContextKey).(*jwt.Token) + claims := jwtinf.Claims.(*AuthClaims) + + if claims.Type == AuthTypeLinkShare { + oldShare, err := getLinkShareFromClaims(claims) + if err != nil { + return "", err + } + share := &models.LinkSharing{} + share.ID = oldShare.ID + err = share.ReadOne(s) + if err != nil { + return "", err + } + return NewLinkShareJWTAuthtoken(share) + } + if claims.Type == AuthTypeUser { + oldUser, err := getUserFromClaims(claims) + if err != nil { + return "", err + } + u, err := user.GetUserWithEmail(s, &user.User{ID: oldUser.ID}) + if err != nil { + return "", err + } + return NewUserJWTAuthtoken(u) + } + return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) +} + +// GetJWTConfig returns the config for the default JWT middleware +func GetJWTConfig() middleware.JWTConfig { + return middleware.JWTConfig{ + SigningKey: []byte(config.ServiceJWTSecret.GetString()), + SigningMethod: middleware.AlgorithmHS256, + ContextKey: authTokenContextKey, + Claims: &AuthClaims{}, + } +} + +// GetOrCreateUserFromExternalAuth returns a user after finding or creating a matching user for the provided details +func GetOrCreateUserFromExternalAuth(s *xorm.Session, issuer, subject, email, name, preferredUsername string) (u *user.User, err error) { + if issuer == "" || subject == "" || email == "" { + return nil, echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Missing required data."}) + } + + // Check if the user exists for that issuer and subject + u, err = user.GetUserWithEmail(s, &user.User{ + Issuer: issuer, + Subject: subject, + }) + if err != nil && !user.IsErrUserDoesNotExist(err) { + return nil, err + } + + // If no user exists, create one with the preferred username if it is not already taken + if user.IsErrUserDoesNotExist(err) { + uu := &user.User{ + Username: preferredUsername, + Email: email, + IsActive: true, + Issuer: issuer, + Subject: subject, + } + + // Check if we actually have a preferred username and generate a random one right away if we don't + if uu.Username == "" { + uu.Username = petname.Generate(3, "-") + } + + u, err = user.CreateUser(s, uu) + if err != nil && !user.IsErrUsernameExists(err) { + return nil, err + } + + // If their preferred username is already taken, create some random one from the email and subject + if user.IsErrUsernameExists(err) { + uu.Username = petname.Generate(3, "-") + u, err = user.CreateUser(s, uu) + if err != nil { + return nil, err + } + } + + // And create its namespace + err = models.CreateNewNamespaceForUser(s, u) + if err != nil { + return nil, err + } + + return + } + + // If it exists, check if the email address changed and change it if not + if email != u.Email || (name != "" && name != u.Name) { + if email != u.Email { + u.Email = email + } + if name != "" && name != u.Name { + u.Name = name + } + u, err = user.UpdateUser(s, &user.User{ + ID: u.ID, + Email: u.Email, + Name: u.Name, + Issuer: issuer, + Subject: subject, + }) + if err != nil { + return nil, err + } + } + + return +} diff --git a/pkg/modules/auth/openid/openid_test.go b/pkg/modules/auth/auth_test.go similarity index 74% rename from pkg/modules/auth/openid/openid_test.go rename to pkg/modules/auth/auth_test.go index b21319703..40dd112ce 100644 --- a/pkg/modules/auth/openid/openid_test.go +++ b/pkg/modules/auth/auth_test.go @@ -14,7 +14,7 @@ // You should have received a copy of the GNU Affero General Public Licensee // along with this program. If not, see . -package openid +package auth import ( "testing" @@ -29,18 +29,13 @@ func TestGetOrCreateUser(t *testing.T) { s := db.NewSession() defer s.Close() - cl := &claims{ - Email: "test@example.com", - PreferredUsername: "someUserWhoDoesNotExistYet", - } - u, err := getOrCreateUser(s, cl, "https://some.issuer", "12345") + u, err := GetOrCreateUserFromExternalAuth(s, "https://some.issuer", "12345", "test@example.com", "", "someUserWhoDoesNotExistYet") assert.NoError(t, err) err = s.Commit() - assert.NoError(t, err) db.AssertExists(t, "users", map[string]interface{}{ "id": u.ID, - "email": cl.Email, + "email": "test@example.com", "username": "someUserWhoDoesNotExistYet", }, false) }) @@ -49,11 +44,7 @@ func TestGetOrCreateUser(t *testing.T) { s := db.NewSession() defer s.Close() - cl := &claims{ - Email: "test@example.com", - PreferredUsername: "", - } - u, err := getOrCreateUser(s, cl, "https://some.issuer", "12345") + u, err := GetOrCreateUserFromExternalAuth(s, "https://some.issuer", "12345", "test@example.com", "", "") assert.NoError(t, err) assert.NotEmpty(t, u.Username) err = s.Commit() @@ -61,7 +52,7 @@ func TestGetOrCreateUser(t *testing.T) { db.AssertExists(t, "users", map[string]interface{}{ "id": u.ID, - "email": cl.Email, + "email": "test@example.com", }, false) }) t.Run("new user, no email address", func(t *testing.T) { @@ -69,10 +60,7 @@ func TestGetOrCreateUser(t *testing.T) { s := db.NewSession() defer s.Close() - cl := &claims{ - Email: "", - } - _, err := getOrCreateUser(s, cl, "https://some.issuer", "12345") + _, err := GetOrCreateUserFromExternalAuth(s, "https://some.issuer", "12345", "", "", "") assert.Error(t, err) }) t.Run("existing user, different email address", func(t *testing.T) { @@ -80,17 +68,14 @@ func TestGetOrCreateUser(t *testing.T) { s := db.NewSession() defer s.Close() - cl := &claims{ - Email: "other-email-address@some.service.com", - } - u, err := getOrCreateUser(s, cl, "https://some.service.com", "12345") + u, err := GetOrCreateUserFromExternalAuth(s, "https://some.service.com", "12345", "other-email-address@some.service.com", "", "") assert.NoError(t, err) err = s.Commit() assert.NoError(t, err) db.AssertExists(t, "users", map[string]interface{}{ "id": u.ID, - "email": cl.Email, + "email": "other-email-address@some.service.com", }, false) }) } diff --git a/pkg/modules/auth/identityawareproxy/error.go b/pkg/modules/auth/identityawareproxy/error.go new file mode 100644 index 000000000..1427e2f17 --- /dev/null +++ b/pkg/modules/auth/identityawareproxy/error.go @@ -0,0 +1,135 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package identityawareproxy + +import ( + "fmt" + "net/http" + + "code.vikunja.io/web" +) + +// ErrIAPTokenMissing represents a "IAPTokenMissing" kind of error. +type ErrIAPTokenMissing struct { + Header string +} + +// IsErrIAPTokenMissing checks if an error is a ErrIAPTokenMissing. +func IsErrIAPTokenMissing(err error) bool { + _, ok := err.(ErrIAPTokenMissing) + return ok +} + +func (err ErrIAPTokenMissing) Error() string { + return fmt.Sprintf("No JWT provided by the identity-aware proxy at the header %v", err.Header) +} + +// ErrorCodeIAPTokenMissing holds the unique world-error code of this error +const ErrorCodeIAPTokenMissing = 12001 + +// HTTPError holds the http error description +func (err ErrIAPTokenMissing) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusServiceUnavailable, Code: ErrorCodeIAPTokenMissing, Message: "No authentication provided by the identity-aware proxy."} +} + +// ErrIAPPublicKeysetMissing represents a "IAPPublicKeysetMissing" kind of error. +type ErrIAPPublicKeysetMissing struct { + Url string +} + +// IsErrIAPPublicKeysetMissing checks if an error is a ErrIAPPublicKeysetMissing. +func IsErrIAPPublicKeysetMissing(err error) bool { + _, ok := err.(ErrIAPPublicKeysetMissing) + return ok +} + +func (err ErrIAPPublicKeysetMissing) Error() string { + return fmt.Sprintf("Failed to retrive the identity-aware proxy's signing public key at URL: %s", err.Url) +} + +// ErrorCodeIAPPublicKeysetMissing holds the unique world-error code of this error +const ErrorCodeIAPPublicKeysetMissing = 12002 + +// HTTPError holds the http error description +func (err ErrIAPPublicKeysetMissing) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusServiceUnavailable, Code: ErrorCodeIAPPublicKeysetMissing, Message: "Failed to retrive the identity-aware proxy's signing public keys."} +} + +// ErrIAPUserFrontendMismatch represents a "IAPUserDoesNotMatchFrontendUser" kind of error. +type ErrIAPUserFrontendMismatch struct {} + +// IsErrIAPPublicKeysetMissing checks if an error is a ErrIAPUserFrontendMismatch. +func IsErrIAPUserFrontendMismatch(err error) bool { + _, ok := err.(ErrIAPUserFrontendMismatch) + return ok +} + +func (err ErrIAPUserFrontendMismatch) Error() string { + return "Frontend provided user does not match IAP provided user" +} + +// ErrorCodeIAPPublicKeysetMissing holds the unique world-error code of this error +const ErrorCodeIAPUserFrontendMismatch = 12003 + +// HTTPError holds the http error description +func (err ErrIAPUserFrontendMismatch) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrorCodeIAPUserFrontendMismatch, Message: "Invalid provided jwt."} +} + +// ErrIAPUnsupportedJWTSigningMethod represents a "IAPUnsupportedJWTSigningMethod" kind of error. +type ErrIAPUnsupportedJWTSigningMethod struct { + Method string +} + +// IsErrIAPUnsupportedJWTSigningMethod checks if an error is a ErrIAPUnsupportedJWTSigningMethod. +func IsErrIAPUnsupportedJWTSigningMethod(err error) bool { + _, ok := err.(ErrIAPUnsupportedJWTSigningMethod) + return ok +} + +func (err ErrIAPUnsupportedJWTSigningMethod) Error() string { + return fmt.Sprintf("Unsupported JWT signing method: %s", err.Method) +} + +// ErrorCodeIAPUnsupportedJWTSigningMethod holds the unique world-error code of this error +const ErrorCodeIAPUnsupportedJWTSigningMethod = 12004 + +// HTTPError holds the http error description +func (err ErrIAPUnsupportedJWTSigningMethod) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusInternalServerError, Code: ErrorCodeIAPUnsupportedJWTSigningMethod, Message: "Unsupported JWT signing method."} +} + +// ErrIAPJWTMissingKID represents a "ErrIAPJWTMissingKID" kind of error. +type ErrIAPJWTMissingKID struct {} + +// IsErrIAPUnsupportedJWTSigningMethod checks if an error is a ErrIAPJWTMissingKID. +func IsErrIAPJWTMissingKID(err error) bool { + _, ok := err.(ErrIAPJWTMissingKID) + return ok +} + +func (err ErrIAPJWTMissingKID) Error() string { + return "JWT missing KID" +} + +// ErrorCodeErrIAPJWTMissingKID holds the unique world-error code of this error +const ErrorCodeErrIAPJWTMissingKID = 12005 + +// HTTPError holds the http error description +func (err ErrIAPJWTMissingKID) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusInternalServerError, Code: ErrorCodeErrIAPJWTMissingKID, Message: "JWT missing KID."} +} diff --git a/pkg/modules/auth/identityawareproxy/identityawareproxy.go b/pkg/modules/auth/identityawareproxy/identityawareproxy.go new file mode 100644 index 000000000..ba89eb298 --- /dev/null +++ b/pkg/modules/auth/identityawareproxy/identityawareproxy.go @@ -0,0 +1,165 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package identityawareproxy + +import ( + "fmt" + "time" + + "code.vikunja.io/api/pkg/db" + + "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/modules/auth" + "code.vikunja.io/api/pkg/user" + "code.vikunja.io/web" + "github.com/dgrijalva/jwt-go" + "github.com/labstack/echo/v4" +) + +const IAPClaimsContextKey string = "iapClaims" + +// IAPClaims represents the claims made by the authentication JWT +// passed in by the identity-aware proxy +type IAPClaims struct { + Email string `json:"email"` + Name string `json:"name,omitempty"` + PreferredUsername string `json:"preferred_username,omitempty"` + jwt.StandardClaims +} + +// Auth provider used to allow auth to get a web.Auth from the IAP provided identity +type IAPAuthProvider struct{} + +func init() { + auth.RegisterAuthProvider(auth.AuthTypeIAPUser, IAPAuthProvider{}) +} + +// NewIAPUserJWTAuthtoken generates and signes a new jwt token for a user +// These are intentionally short lived because they can be regenerated at +// any time from the IAP authn information. They are not related to +// session length and are only used to provide user info to the frontend +// and a hint to auth.go to retreive auth data from the IAP. +func NewIAPUserJWTAuthtoken(u *user.User) (token string, err error) { + // Set claims + claims := &auth.AuthClaims{ + Type: auth.AuthTypeIAPUser, + UserID: u.ID, + UserUsername: u.Username, + UserEmail: u.Email, + UserName: u.Name, + StandardClaims: jwt.StandardClaims{ + ExpiresAt: time.Now().Add(time.Minute * 5).Unix(), + }, + } + + t := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + + // Generate encoded token and send it as response. + return t.SignedString([]byte(config.ServiceJWTSecret.GetString())) +} + +// Token generates a local, short-lived JWT based on the identity from the identity-aware proxy. +// See also the docs for NewIAPUserJWTAuthtoken +// @Summary Authenticate a user from the Identity-Aware Proxy +// @Description Generates a short-lived JWT based on the identity from the identity-aware proxy in order to provide the front-end with user id and username info +// @tags auth +// @Accept N/A +// @Produce json +// @Success 200 {object} auth.Token +// @Failure 500 {object} models.Message "Internal error" +// @Router /auth/identityawareproxy/token [get] +func GetToken(c echo.Context) error { + cl := c.Get(IAPClaimsContextKey).(*IAPClaims) + + s := db.NewSession() + defer s.Close() + + // Check if we have seen this user before + u, err := auth.GetOrCreateUserFromExternalAuth(s, cl.Issuer, cl.Subject, cl.Email, cl.Name, cl.PreferredUsername) + if err != nil { + _ = s.Rollback() + return err + } + + err = s.Commit() + if err != nil { + return err + } + + // Create token + userToken, err := NewIAPUserJWTAuthtoken(u) + if err != nil { + return err + } + return auth.NewTokenResponse(userToken, c) +} + +// Get a web.Auth object from the identity that the IAP provides +func (p IAPAuthProvider) GetWebAuth(c echo.Context, authClaims *auth.AuthClaims) (web.Auth, error) { + s := db.NewSession() + defer s.Close() + + // Get the user from the IAP identity + cl := c.Get(IAPClaimsContextKey).(*IAPClaims) + u, err := auth.GetOrCreateUserFromExternalAuth(s, cl.Issuer, cl.Subject, cl.Email, cl.Name, cl.PreferredUsername) + if err != nil { + _ = s.Rollback() + return nil, err + } + + err = s.Commit() + if err != nil { + return nil, err + } + + // Sanity check that the user the frontend thinks it has (the authClaims from the JWT it passed in) + // is the same as the user provided by the IAP. + if authClaims.UserID != u.ID { + return nil, ErrIAPUserFrontendMismatch{} + } + + return u, nil +} + +// Validates the claims in the jwt +// Matches the jwt-go Claims interface +func (c *IAPClaims) Valid() error { + // Validate that expiresAt and issuedAt are set and valid (with up to 1 minute of skew) + now := TimeFunc() + skew := time.Minute + if c.VerifyExpiresAt(now.Add(-skew).Unix(), true) == false { + delta := now.Sub(time.Unix(c.ExpiresAt, 0)) + return fmt.Errorf("token is expired by %v", delta) + } + if c.VerifyIssuedAt(now.Add(skew).Unix(), true) == false { + return fmt.Errorf("token used before issued") + + } + + // Validate that subject, email, and issuer are all set + if c.Subject == "" { + return fmt.Errorf("token missing subject") + } + if c.Email == "" { + return fmt.Errorf("token missing email") + } + if c.Issuer == "" { + return fmt.Errorf("token missing issuer") + } + + return nil +} diff --git a/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go b/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go new file mode 100644 index 000000000..91f0d86fd --- /dev/null +++ b/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go @@ -0,0 +1,19 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package identityawareproxy + +// TODO test GetWebAuth() \ No newline at end of file diff --git a/pkg/modules/auth/identityawareproxy/middleware.go b/pkg/modules/auth/identityawareproxy/middleware.go new file mode 100644 index 000000000..a700a296e --- /dev/null +++ b/pkg/modules/auth/identityawareproxy/middleware.go @@ -0,0 +1,132 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package identityawareproxy + +import ( + "fmt" + "sync" + "time" + + "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/log" + "code.vikunja.io/web/handler" + "github.com/dgrijalva/jwt-go" + "github.com/labstack/echo/v4" + "github.com/lestrrat-go/jwx/jwk" +) + +// TimeFunc provides the current time to help validate "exp" and "iss" claims in a JWT. +// It is overridden in unit tests +var TimeFunc = time.Now + +// Caches the public keys of the identity-aware proxy used to validate the auth data it sends +type iapCache struct { + keyset *jwk.Set + mutex sync.Mutex +} + +// GetKeyset returns the cached public keys from the identity-aware proxy +// or fetches them for the first time. +func (cache *iapCache) GetKeyset() (*jwk.Set, error) { + if cache.keyset != nil { + return cache.keyset, nil + } + + cache.mutex.Lock() + defer cache.mutex.Unlock() + + // Check that another thread has not fetched the keyset + if cache.keyset != nil { + return cache.keyset, nil + } + + // Fetch the public key(s) from the identity-aware proxy + keyset, err := jwk.FetchHTTP(config.AuthIdentityAwareProxyJwksUri.GetString()) + if err != nil { + log.Error("Failed to retrive the identity-aware proxy's signing public key at URL %s: %v", config.AuthIdentityAwareProxyJwksUri.GetString(), err) + return nil, ErrIAPPublicKeysetMissing{Url: config.AuthIdentityAwareProxyJwksUri.GetString()} + } + cache.keyset = keyset + return cache.keyset, nil +} + +// The identity-aware proxy authentication middleware parses and validates the +// JWT provided by the IAP +func Middleware() echo.MiddlewareFunc { + cache := &iapCache{} + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + token := c.Request().Header.Get(config.AuthIdentityAwareProxyJwtHeader.GetString()) + if token == "" { + err := ErrIAPTokenMissing{Header: config.AuthIdentityAwareProxyJwtHeader.GetString()} + return handler.HandleHTTPError(err, c) + } + + keyset, err := cache.GetKeyset() + if err != nil { + return handler.HandleHTTPError(err, c) + } + + cl, err := parseAndValidateJwt(token, keyset) + if err != nil { + return handler.HandleHTTPError(err, c) + } + + c.Set(IAPClaimsContextKey, cl) + return next(c) + } + } +} + +func parseAndValidateJwt(token string, keyset *jwk.Set) (*IAPClaims, error) { + // Parse the jwt from the identity-aware proxy using the correct key + tken, err := jwt.ParseWithClaims(token, &IAPClaims{}, func(unvalidatedToken *jwt.Token) (interface{}, error) { + // Only support either ECDSA or RSA signing methods. Never support the "none" signing method + if _, ok := unvalidatedToken.Method.(*jwt.SigningMethodECDSA); !ok { + if _, ok := unvalidatedToken.Method.(*jwt.SigningMethodRSA); !ok { + return nil, ErrIAPUnsupportedJWTSigningMethod{Method: unvalidatedToken.Header["alg"].(string)} + } + } + + keyID, ok := unvalidatedToken.Header["kid"].(string) + if !ok { + return nil, ErrIAPJWTMissingKID{} + } + + keys := keyset.LookupKeyID(keyID) + if len(keys) != 1 { + return nil, ErrIAPJWTMissingKID{} + } + + var rawkey interface{} // This is the raw key, like *rsa.PublicKey or *ecdsa.PublicKey + if err := keys[0].Raw(&rawkey); err != nil { + return nil, err + } + + return jwk.PublicKeyOf(rawkey) + }) + if err != nil { + return nil, err + } + + cl, ok := tken.Claims.(*IAPClaims) + if !ok || !tken.Valid { + return nil, fmt.Errorf("failed to parse the jwt claims") + } + + return cl, nil +} diff --git a/pkg/modules/auth/identityawareproxy/middleware_test.go b/pkg/modules/auth/identityawareproxy/middleware_test.go new file mode 100644 index 000000000..1ec3062bf --- /dev/null +++ b/pkg/modules/auth/identityawareproxy/middleware_test.go @@ -0,0 +1,130 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package identityawareproxy + +import ( + "testing" + "time" + + "github.com/dgrijalva/jwt-go" + "github.com/lestrrat-go/jwx/jwk" + "github.com/stretchr/testify/assert" +) + +/* Valid token with kid "sig-1606406403" and the following claims: +{ + "aud": "vikunja-url", + "email": "test@example.com", + "exp": 1000, + "iat": 1, + "iss": "proxy-url", + "sub": "12345", + "user": "12345" +}*/ +var validToken = "eyJhbGciOiJFUzI1NiIsImtpZCI6InNpZy0xNjA2NDA2NDAzIiwidHlwIjoiSldUIn0.eyJhdWQiOiJ2aWt1bmphLXVybCIsImVtYWlsIjoidGVzdEBleGFtcGxlLmNvbSIsImV4cCI6MTAwMCwiaWF0IjoxLCJpc3MiOiJwcm94eS11cmwiLCJzdWIiOiIxMjM0NSIsInVzZXIiOiIxMjM0NSJ9.Shh0wxVaojGV2U2FQpgWgMjvU8QbjQSZcN062Qd-WShyyPG_vZsJwbBV6EVM5v_HdN2uMJN0HtEELEPBEM7Hiw" + +var validJwks = `{"keys":[{ + "crv":"P-256", + "kty":"EC", + "use":"sig", + "kid": "sig-1606406403", + "x":"vejjjK-FfBOLU_Vz0t12dx1zmfdR2GseEnverHOKJKk", + "y":"09pkYKAr51w8-k5s37_M9oBDU8nI4ALkLufTEumO-r4", + "alg": "ES256" +}]}` + +// Override time value for tests. Restore default value after. +func at(t time.Time, f func()) { + TimeFunc = func() time.Time { + return t + } + f() + TimeFunc = time.Now +} + +func TestParseAndValidateJwt(t *testing.T) { + // Run the test at a valid time for the JWT expiration + at(time.Unix(50, 0), func() { + t.Run("valid jwt and key within expiration", func(t *testing.T) { + validKeySet, _ := jwk.ParseString(validJwks) + cl, err := parseAndValidateJwt(validToken, validKeySet) + assert.NoError(t, err) + assert.Equal(t, &IAPClaims{ + Email: "test@example.com", + Name: "", + PreferredUsername: "", + StandardClaims: jwt.StandardClaims{ + Audience: "vikunja-url", + ExpiresAt: 1000, + Id: "", + IssuedAt: 1, + Issuer: "proxy-url", + NotBefore: 0, + Subject: "12345", + }, + }, cl) + }) + }) + + // Run the test within the skew of the the expiration + at(time.Unix(1059, 0), func() { + t.Run("valid jwt and key past expiration within skew", func(t *testing.T) { + validKeySet, _ := jwk.ParseString(validJwks) + cl, err := parseAndValidateJwt(validToken, validKeySet) + assert.NoError(t, err) + assert.Equal(t, &IAPClaims{ + Email: "test@example.com", + Name: "", + PreferredUsername: "", + StandardClaims: jwt.StandardClaims{ + Audience: "vikunja-url", + ExpiresAt: 1000, + Id: "", + IssuedAt: 1, + Issuer: "proxy-url", + NotBefore: 0, + Subject: "12345", + }, + }, cl) + }) + }) + + // Run the test outside the skew of the the expiration + at(time.Unix(1061, 0), func() { + t.Run("expired jwt", func(t *testing.T) { + validKeySet, _ := jwk.ParseString(validJwks) + cl, err := parseAndValidateJwt(validToken, validKeySet) + assert.Nil(t, cl) + assert.EqualError(t, err, "token is expired by 1m1s") + }) + }) + + t.Run("missing key", func(t *testing.T) { + keySet, _ := jwk.ParseString(`{"keys":[{ + "crv":"P-256", + "kty":"EC", + "use":"sig", + "kid": "non-matching-sig", + "x":"vejjjK-FfBOLU_Vz0t12dx1zmfdR2GseEnverHOKJKk", + "y":"09pkYKAr51w8-k5s37_M9oBDU8nI4ALkLufTEumO-r4", + "alg": "ES256" + }]}`) + cl, err := parseAndValidateJwt(validToken, keySet) + assert.EqualError(t, err, "JWT missing KID") + assert.Nil(t, cl) + }) +} diff --git a/pkg/modules/auth/openid/main_test.go b/pkg/modules/auth/main_test.go similarity index 98% rename from pkg/modules/auth/openid/main_test.go rename to pkg/modules/auth/main_test.go index 21b51d68e..85a8bdac7 100644 --- a/pkg/modules/auth/openid/main_test.go +++ b/pkg/modules/auth/main_test.go @@ -14,7 +14,7 @@ // You should have received a copy of the GNU Affero General Public Licensee // along with this program. If not, see . -package openid +package auth import ( "os" diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 8cd169bd3..8ba382aea 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -26,14 +26,11 @@ import ( "code.vikunja.io/web/handler" "code.vikunja.io/api/pkg/db" - "xorm.io/xorm" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" - "code.vikunja.io/api/pkg/user" "github.com/coreos/go-oidc" - petname "github.com/dustinkirkland/golang-petname" "github.com/labstack/echo/v4" "golang.org/x/oauth2" ) @@ -147,7 +144,7 @@ func HandleCallback(c echo.Context) error { defer s.Close() // Check if we have seen this user before - u, err := getOrCreateUser(s, cl, idToken.Issuer, idToken.Subject) + u, err := auth.GetOrCreateUserFromExternalAuth(s, idToken.Issuer, idToken.Subject, cl.Email, cl.Name, cl.PreferredUsername) if err != nil { _ = s.Rollback() log.Errorf("Error creating new user for provider %s: %v", provider.Name, err) @@ -162,74 +159,3 @@ func HandleCallback(c echo.Context) error { // Create token return auth.NewUserAuthTokenResponse(u, c) } - -func getOrCreateUser(s *xorm.Session, cl *claims, issuer, subject string) (u *user.User, err error) { - // Check if the user exists for that issuer and subject - u, err = user.GetUserWithEmail(s, &user.User{ - Issuer: issuer, - Subject: subject, - }) - if err != nil && !user.IsErrUserDoesNotExist(err) { - return nil, err - } - - // If no user exists, create one with the preferred username if it is not already taken - if user.IsErrUserDoesNotExist(err) { - uu := &user.User{ - Username: cl.PreferredUsername, - Email: cl.Email, - IsActive: true, - Issuer: issuer, - Subject: subject, - } - - // Check if we actually have a preferred username and generate a random one right away if we don't - if uu.Username == "" { - uu.Username = petname.Generate(3, "-") - } - - u, err = user.CreateUser(s, uu) - if err != nil && !user.IsErrUsernameExists(err) { - return nil, err - } - - // If their preferred username is already taken, create some random one from the email and subject - if user.IsErrUsernameExists(err) { - uu.Username = petname.Generate(3, "-") - u, err = user.CreateUser(s, uu) - if err != nil { - return nil, err - } - } - - // And create its namespace - err = models.CreateNewNamespaceForUser(s, u) - if err != nil { - return nil, err - } - - return - } - - // If it exists, check if the email address changed and change it if not - if cl.Email != u.Email || cl.Name != u.Name { - if cl.Email != u.Email { - u.Email = cl.Email - } - if cl.Name != u.Name { - u.Name = cl.Name - } - u, err = user.UpdateUser(s, &user.User{ - ID: u.ID, - Email: u.Email, - Name: u.Name, - Issuer: issuer, - Subject: subject, - }) - if err != nil { - return nil, err - } - } - - return -} diff --git a/pkg/modules/migration/handler/handler.go b/pkg/modules/migration/handler/handler.go index 158f680a2..d890e6721 100644 --- a/pkg/modules/migration/handler/handler.go +++ b/pkg/modules/migration/handler/handler.go @@ -20,8 +20,8 @@ import ( "net/http" "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/modules/migration" - user2 "code.vikunja.io/api/pkg/user" "code.vikunja.io/web/handler" "github.com/labstack/echo/v4" ) @@ -55,7 +55,7 @@ func (mw *MigrationWeb) Migrate(c echo.Context) error { ms := mw.MigrationStruct() // Get the user from context - user, err := user2.GetCurrentUser(c) + user, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } @@ -84,7 +84,7 @@ func (mw *MigrationWeb) Migrate(c echo.Context) error { func (mw *MigrationWeb) Status(c echo.Context) error { ms := mw.MigrationStruct() - user, err := user2.GetCurrentUser(c) + user, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } diff --git a/pkg/routes/api/v1/avatar.go b/pkg/routes/api/v1/avatar.go index 03c9e0576..68dbdb12b 100644 --- a/pkg/routes/api/v1/avatar.go +++ b/pkg/routes/api/v1/avatar.go @@ -21,6 +21,7 @@ import ( "code.vikunja.io/api/pkg/files" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/modules/avatar" "code.vikunja.io/api/pkg/modules/avatar/empty" "code.vikunja.io/api/pkg/modules/avatar/gravatar" @@ -123,7 +124,7 @@ func UploadAvatar(c echo.Context) (err error) { s := db.NewSession() defer s.Close() - uc, err := user.GetCurrentUser(c) + uc, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } diff --git a/pkg/routes/api/v1/info.go b/pkg/routes/api/v1/info.go index e7ef3df35..7c6f1c5de 100644 --- a/pkg/routes/api/v1/info.go +++ b/pkg/routes/api/v1/info.go @@ -51,8 +51,9 @@ type vikunjaInfos struct { } type authInfo struct { - Local localAuthInfo `json:"local"` - OpenIDConnect openIDAuthInfo `json:"openid_connect"` + Local localAuthInfo `json:"local"` + OpenIDConnect openIDAuthInfo `json:"openid_connect"` + IdentityAwareProxy identityAwareProxyAuthInfo `json:"identity_aware_proxy"` } type localAuthInfo struct { @@ -65,6 +66,10 @@ type openIDAuthInfo struct { Providers []*openid.Provider `json:"providers"` } +type identityAwareProxyAuthInfo struct { + Enabled bool `json:"enabled"` +} + type legalInfo struct { ImprintURL string `json:"imprint_url"` PrivacyPolicyURL string `json:"privacy_policy_url"` @@ -101,6 +106,9 @@ func Info(c echo.Context) error { Enabled: config.AuthOpenIDEnabled.GetBool(), RedirectURL: config.AuthOpenIDRedirectURL.GetString(), }, + IdentityAwareProxy: identityAwareProxyAuthInfo{ + Enabled: config.AuthIdentityAwareProxyEnabled.GetBool(), + }, }, } diff --git a/pkg/routes/api/v1/list_by_namespace.go b/pkg/routes/api/v1/list_by_namespace.go index caebee692..0edb6ec47 100644 --- a/pkg/routes/api/v1/list_by_namespace.go +++ b/pkg/routes/api/v1/list_by_namespace.go @@ -24,7 +24,7 @@ import ( "xorm.io/xorm" "code.vikunja.io/api/pkg/models" - "code.vikunja.io/api/pkg/user" + "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/web/handler" "github.com/labstack/echo/v4" ) @@ -54,7 +54,7 @@ func GetListsByNamespaceID(c echo.Context) error { } // Get the lists - doer, err := user.GetCurrentUser(c) + doer, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } @@ -81,7 +81,7 @@ func getNamespace(s *xorm.Session, c echo.Context) (namespace *models.Namespace, } // Check if the user has acces to that namespace - u, err := user.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return } diff --git a/pkg/routes/api/v1/login.go b/pkg/routes/api/v1/login.go index 6a5ae7a89..33898855f 100644 --- a/pkg/routes/api/v1/login.go +++ b/pkg/routes/api/v1/login.go @@ -25,7 +25,6 @@ import ( "code.vikunja.io/api/pkg/modules/auth" user2 "code.vikunja.io/api/pkg/user" "code.vikunja.io/web/handler" - "github.com/dgrijalva/jwt-go" "github.com/labstack/echo/v4" ) @@ -94,39 +93,13 @@ func Login(c echo.Context) error { // @Failure 400 {object} models.Message "Only user token are available for renew." // @Router /user/token [post] func RenewToken(c echo.Context) (err error) { - s := db.NewSession() defer s.Close() - jwtinf := c.Get("user").(*jwt.Token) - claims := jwtinf.Claims.(jwt.MapClaims) - typ := int(claims["type"].(float64)) - if typ == auth.AuthTypeLinkShare { - share := &models.LinkSharing{} - share.ID = int64(claims["id"].(float64)) - err := share.ReadOne(s, share) - if err != nil { - _ = s.Rollback() - return handler.HandleHTTPError(err, c) - } - t, err := auth.NewLinkShareJWTAuthtoken(share) - if err != nil { - _ = s.Rollback() - return handler.HandleHTTPError(err, c) - } - return c.JSON(http.StatusOK, auth.Token{Token: t}) - } - - u, err := user2.GetUserFromClaims(claims) + newToken, err := auth.RenewToken(s, c) if err != nil { _ = s.Rollback() - return handler.HandleHTTPError(err, c) - } - - user, err := user2.GetUserWithEmail(s, &user2.User{ID: u.ID}) - if err != nil { - _ = s.Rollback() - return handler.HandleHTTPError(err, c) + return handler.HandleHTTPError(err, c) } if err := s.Commit(); err != nil { @@ -134,6 +107,5 @@ func RenewToken(c echo.Context) (err error) { return handler.HandleHTTPError(err, c) } - // Create token - return auth.NewUserAuthTokenResponse(user, c) + return auth.NewTokenResponse(newToken, c) } diff --git a/pkg/routes/api/v1/token_check.go b/pkg/routes/api/v1/token_check.go index c3cd263bc..f63b55614 100644 --- a/pkg/routes/api/v1/token_check.go +++ b/pkg/routes/api/v1/token_check.go @@ -20,16 +20,15 @@ import ( "fmt" "code.vikunja.io/api/pkg/models" - "github.com/dgrijalva/jwt-go" + "code.vikunja.io/api/pkg/modules/auth" "github.com/labstack/echo/v4" ) // CheckToken checks prints a message if the token is valid or not. Currently only used for testing pourposes. func CheckToken(c echo.Context) error { + auth, err := auth.GetAuthFromClaims(c) - user := c.Get("user").(*jwt.Token) - - fmt.Println(user.Valid) + fmt.Println(auth != nil && err == nil) return c.JSON(418, models.Message{Message: "🍵"}) } diff --git a/pkg/routes/api/v1/user_settings.go b/pkg/routes/api/v1/user_settings.go index e5255efe8..acec8d1e6 100644 --- a/pkg/routes/api/v1/user_settings.go +++ b/pkg/routes/api/v1/user_settings.go @@ -22,6 +22,7 @@ import ( "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth" user2 "code.vikunja.io/api/pkg/user" "code.vikunja.io/web/handler" "github.com/labstack/echo/v4" @@ -60,7 +61,7 @@ type UserSettings struct { // @Router /user/settings/avatar [get] func GetUserAvatarProvider(c echo.Context) error { - u, err := user2.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } @@ -103,7 +104,7 @@ func ChangeUserAvatarProvider(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "Bad avatar type provided.") } - u, err := user2.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } @@ -151,7 +152,7 @@ func UpdateGeneralUserSettings(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "Bad user name provided.") } - u, err := user2.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } diff --git a/pkg/routes/api/v1/user_totp.go b/pkg/routes/api/v1/user_totp.go index 7996d1f95..db38c574a 100644 --- a/pkg/routes/api/v1/user_totp.go +++ b/pkg/routes/api/v1/user_totp.go @@ -26,6 +26,7 @@ import ( "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/user" "code.vikunja.io/web/handler" "github.com/labstack/echo/v4" @@ -44,7 +45,7 @@ import ( // @Failure 500 {object} models.Message "Internal server error." // @Router /user/settings/totp/enroll [post] func UserTOTPEnroll(c echo.Context) error { - u, err := user.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } @@ -81,7 +82,7 @@ func UserTOTPEnroll(c echo.Context) error { // @Failure 500 {object} models.Message "Internal server error." // @Router /user/settings/totp/enable [post] func UserTOTPEnable(c echo.Context) error { - u, err := user.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } @@ -137,7 +138,7 @@ func UserTOTPDisable(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "Invalid model provided.") } - u, err := user.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } @@ -182,7 +183,7 @@ func UserTOTPDisable(c echo.Context) error { // @Failure 500 {object} models.Message "Internal server error." // @Router /user/settings/totp/qrcode [get] func UserTOTPQrCode(c echo.Context) error { - u, err := user.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } @@ -222,7 +223,7 @@ func UserTOTPQrCode(c echo.Context) error { // @Failure 500 {object} models.Message "Internal server error." // @Router /user/settings/totp [get] func UserTOTP(c echo.Context) error { - u, err := user.GetCurrentUser(c) + u, err := auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } diff --git a/pkg/routes/api/v1/user_update_email.go b/pkg/routes/api/v1/user_update_email.go index 52b4b5eea..9fd47659a 100644 --- a/pkg/routes/api/v1/user_update_email.go +++ b/pkg/routes/api/v1/user_update_email.go @@ -24,6 +24,7 @@ import ( "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/user" "code.vikunja.io/web/handler" "github.com/labstack/echo/v4" @@ -53,7 +54,7 @@ func UpdateUserEmail(c echo.Context) (err error) { return echo.NewHTTPError(http.StatusBadRequest, "Invalid model provided.") } - emailUpdate.User, err = user.GetCurrentUser(c) + emailUpdate.User, err = auth.GetCurrentUser(c) if err != nil { return handler.HandleHTTPError(err, c) } diff --git a/pkg/routes/api/v1/user_update_password.go b/pkg/routes/api/v1/user_update_password.go index b645ef0b4..dd6b9901f 100644 --- a/pkg/routes/api/v1/user_update_password.go +++ b/pkg/routes/api/v1/user_update_password.go @@ -22,6 +22,7 @@ import ( "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/user" "code.vikunja.io/web/handler" "github.com/labstack/echo/v4" @@ -48,7 +49,7 @@ type UserPassword struct { // @Router /user/password [post] func UserChangePassword(c echo.Context) error { // Check if the user is itself - doer, err := user.GetCurrentUser(c) + doer, err := auth.GetCurrentUser(c) if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, "Error getting current user.") } diff --git a/pkg/routes/routes.go b/pkg/routes/routes.go index 7232fe622..3cdc88b31 100644 --- a/pkg/routes/routes.go +++ b/pkg/routes/routes.go @@ -55,6 +55,7 @@ import ( "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" + "code.vikunja.io/api/pkg/modules/auth/identityawareproxy" "code.vikunja.io/api/pkg/modules/auth/openid" "code.vikunja.io/api/pkg/modules/background" backgroundHandler "code.vikunja.io/api/pkg/modules/background/handler" @@ -239,11 +240,6 @@ func registerAPIRoutes(a *echo.Group) { n.POST("/auth/openid/:provider/callback", openid.HandleCallback) } - // Testing - if config.ServiceTestingtoken.GetString() != "" { - n.PATCH("/test/:table", apiv1.HandleTesting) - } - // Info endpoint n.GET("/info", apiv1.Info) @@ -255,9 +251,19 @@ func registerAPIRoutes(a *echo.Group) { n.POST("/shares/:share/auth", apiv1.AuthenticateLinkShare) } - // ===== Routes with Authetication ===== - // Authetification - a.Use(middleware.JWT([]byte(config.ServiceJWTSecret.GetString()))) + // Identity-Aware Proxy auth, requires the same iap middleware as authenticated routes + if config.AuthIdentityAwareProxyEnabled.GetBool() { + m := n.Group("") + m.Use(identityawareproxy.Middleware()) + m.GET("/auth/identityawareproxy/token", identityawareproxy.GetToken) + } + + // ===== Routes with Authentication ===== + // Authentification + a.Use(middleware.JWTWithConfig(auth.GetJWTConfig())) + if config.AuthIdentityAwareProxyEnabled.GetBool() { + a.Use(identityawareproxy.Middleware()) + } // Rate limit setupRateLimit(a, config.RateLimitKind.GetString()) diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index 647500357..34eacccd9 100644 --- a/pkg/swagger/docs.go +++ b/pkg/swagger/docs.go @@ -32,6 +32,40 @@ var doc = `{ "host": "{{.Host}}", "basePath": "{{.BasePath}}", "paths": { + "/auth/identityawareproxy/login": { + "post": { + "security": [ + { + "JWTKeyAuth": [] + } + ], + "description": "After a redirect from the OpenID Connect provider to the frontend has been made with the authentication ` + "`" + `code` + "`" + `, this endpoint can be used to obtain a jwt token for that user and thus log them in.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "auth" + ], + "summary": "Authenticate a user with OpenID Connect", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/auth.Token" + } + }, + "500": { + "description": "Internal error", + "schema": { + "$ref": "#/definitions/models.Message" + } + } + } + } + }, "/auth/openid/{provider}/callback": { "post": { "security": [ @@ -8581,6 +8615,9 @@ var doc = `{ "v1.authInfo": { "type": "object", "properties": { + "identity_aware_proxy": { + "$ref": "#/definitions/v1.identityAwareProxyAuthInfo" + }, "local": { "$ref": "#/definitions/v1.localAuthInfo" }, @@ -8589,6 +8626,17 @@ var doc = `{ } } }, + "v1.identityAwareProxyAuthInfo": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "name": { + "type": "string" + } + } + }, "v1.legalInfo": { "type": "object", "properties": { diff --git a/pkg/swagger/swagger.json b/pkg/swagger/swagger.json index 90e4a2bac..64ad5d212 100644 --- a/pkg/swagger/swagger.json +++ b/pkg/swagger/swagger.json @@ -15,6 +15,40 @@ }, "basePath": "/api/v1", "paths": { + "/auth/identityawareproxy/login": { + "post": { + "security": [ + { + "JWTKeyAuth": [] + } + ], + "description": "After a redirect from the OpenID Connect provider to the frontend has been made with the authentication `code`, this endpoint can be used to obtain a jwt token for that user and thus log them in.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "auth" + ], + "summary": "Authenticate a user with OpenID Connect", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/auth.Token" + } + }, + "500": { + "description": "Internal error", + "schema": { + "$ref": "#/definitions/models.Message" + } + } + } + } + }, "/auth/openid/{provider}/callback": { "post": { "security": [ @@ -8564,6 +8598,9 @@ "v1.authInfo": { "type": "object", "properties": { + "identity_aware_proxy": { + "$ref": "#/definitions/v1.identityAwareProxyAuthInfo" + }, "local": { "$ref": "#/definitions/v1.localAuthInfo" }, @@ -8572,6 +8609,17 @@ } } }, + "v1.identityAwareProxyAuthInfo": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "name": { + "type": "string" + } + } + }, "v1.legalInfo": { "type": "object", "properties": { diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index c52ad481f..f6d8afe33 100644 --- a/pkg/swagger/swagger.yaml +++ b/pkg/swagger/swagger.yaml @@ -1199,11 +1199,20 @@ definitions: type: object v1.authInfo: properties: + identity_aware_proxy: + $ref: '#/definitions/v1.identityAwareProxyAuthInfo' local: $ref: '#/definitions/v1.localAuthInfo' openid_connect: $ref: '#/definitions/v1.openIDAuthInfo' type: object + v1.identityAwareProxyAuthInfo: + properties: + enabled: + type: boolean + name: + type: string + type: object v1.legalInfo: properties: imprint_url: @@ -1329,6 +1338,27 @@ paths: summary: User Avatar tags: - user + /auth/identityawareproxy/login: + post: + consumes: + - application/json + description: After a redirect from the OpenID Connect provider to the frontend has been made with the authentication `code`, this endpoint can be used to obtain a jwt token for that user and thus log them in. + produces: + - application/json + responses: + "200": + description: OK + schema: + $ref: '#/definitions/auth.Token' + "500": + description: Internal error + schema: + $ref: '#/definitions/models.Message' + security: + - JWTKeyAuth: [] + summary: Authenticate a user with OpenID Connect + tags: + - auth /auth/openid/{provider}/callback: post: consumes: diff --git a/pkg/user/user.go b/pkg/user/user.go index 8b32510a2..caea1a348 100644 --- a/pkg/user/user.go +++ b/pkg/user/user.go @@ -29,8 +29,6 @@ import ( "xorm.io/xorm" "code.vikunja.io/web" - "github.com/dgrijalva/jwt-go" - "github.com/labstack/echo/v4" "golang.org/x/crypto/bcrypt" ) @@ -279,29 +277,6 @@ func CheckUserPassword(user *User, password string) error { return nil } -// GetCurrentUser returns the current user based on its jwt token -func GetCurrentUser(c echo.Context) (user *User, err error) { - jwtinf := c.Get("user").(*jwt.Token) - claims := jwtinf.Claims.(jwt.MapClaims) - return GetUserFromClaims(claims) -} - -// GetUserFromClaims Returns a new user from jwt claims -func GetUserFromClaims(claims jwt.MapClaims) (user *User, err error) { - userID, ok := claims["id"].(float64) - if !ok { - return user, ErrCouldNotGetUserID{} - } - user = &User{ - ID: int64(userID), - Email: claims["email"].(string), - Username: claims["username"].(string), - Name: claims["name"].(string), - } - - return -} - // UpdateUser updates a user func UpdateUser(s *xorm.Session, user *User) (updatedUser *User, err error) { -- 2.40.1 From 27891e44924e1b96f0eaa3054846012ec20e0f95 Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Mon, 28 Dec 2020 15:05:28 -0500 Subject: [PATCH 2/9] Docs and formatting --- docs/content/doc/setup/config.md | 5 ++--- pkg/modules/auth/auth.go | 12 ++++++------ pkg/modules/auth/identityawareproxy/error.go | 4 ++-- .../identityawareproxy_test.go | 2 +- pkg/routes/api/v1/info.go | 2 +- pkg/routes/api/v1/login.go | 2 +- pkg/swagger/docs.go | 18 +++++------------- pkg/swagger/swagger.json | 18 +++++------------- pkg/swagger/swagger.yaml | 14 +++++--------- 9 files changed, 28 insertions(+), 49 deletions(-) diff --git a/docs/content/doc/setup/config.md b/docs/content/doc/setup/config.md index d4fdaedb5..b78ccf2f6 100644 --- a/docs/content/doc/setup/config.md +++ b/docs/content/doc/setup/config.md @@ -654,8 +654,7 @@ Default: `` Identity-Aware Proxy configuration will allow users to authenticate through an identity-aware proxy in front of Vikunja.
The proxy needs to supply a signed JWT over HTTP headers with claims for `sub`, `iss`, and `email`. -This JWT must be signed using ECDSA or RSA. It must also serve the public keys for that signed JWT in the JWKS format. -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 this authentication. -Default: `` \ No newline at end of file +Default: `` + diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index 4f0d2aa44..139544382 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -106,11 +106,11 @@ func NewUserAuthTokenResponse(u *user.User, c echo.Context) error { func NewUserJWTAuthtoken(u *user.User) (token string, err error) { // Set claims claims := &AuthClaims{ - Type: AuthTypeUser, - UserID: u.ID, - UserUsername: u.Username, - UserEmail: u.Email, - UserName: u.Name, + Type: AuthTypeUser, + UserID: u.ID, + UserUsername: u.Username, + UserEmail: u.Email, + UserName: u.Name, UserEmailRemindersEnabled: u.EmailRemindersEnabled, StandardClaims: jwt.StandardClaims{ ExpiresAt: time.Now().Add(time.Hour * 72).Unix(), @@ -249,7 +249,7 @@ func GetJWTConfig() middleware.JWTConfig { // GetOrCreateUserFromExternalAuth returns a user after finding or creating a matching user for the provided details func GetOrCreateUserFromExternalAuth(s *xorm.Session, issuer, subject, email, name, preferredUsername string) (u *user.User, err error) { if issuer == "" || subject == "" || email == "" { - return nil, echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Missing required data."}) + return nil, echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Missing required data."}) } // Check if the user exists for that issuer and subject diff --git a/pkg/modules/auth/identityawareproxy/error.go b/pkg/modules/auth/identityawareproxy/error.go index 1427e2f17..8febb04bb 100644 --- a/pkg/modules/auth/identityawareproxy/error.go +++ b/pkg/modules/auth/identityawareproxy/error.go @@ -70,7 +70,7 @@ func (err ErrIAPPublicKeysetMissing) HTTPError() web.HTTPError { } // ErrIAPUserFrontendMismatch represents a "IAPUserDoesNotMatchFrontendUser" kind of error. -type ErrIAPUserFrontendMismatch struct {} +type ErrIAPUserFrontendMismatch struct{} // IsErrIAPPublicKeysetMissing checks if an error is a ErrIAPUserFrontendMismatch. func IsErrIAPUserFrontendMismatch(err error) bool { @@ -114,7 +114,7 @@ func (err ErrIAPUnsupportedJWTSigningMethod) HTTPError() web.HTTPError { } // ErrIAPJWTMissingKID represents a "ErrIAPJWTMissingKID" kind of error. -type ErrIAPJWTMissingKID struct {} +type ErrIAPJWTMissingKID struct{} // IsErrIAPUnsupportedJWTSigningMethod checks if an error is a ErrIAPJWTMissingKID. func IsErrIAPJWTMissingKID(err error) bool { diff --git a/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go b/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go index 91f0d86fd..55a28ce4c 100644 --- a/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go +++ b/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go @@ -16,4 +16,4 @@ package identityawareproxy -// TODO test GetWebAuth() \ No newline at end of file +// TODO test GetWebAuth() diff --git a/pkg/routes/api/v1/info.go b/pkg/routes/api/v1/info.go index 7c6f1c5de..2ee291d79 100644 --- a/pkg/routes/api/v1/info.go +++ b/pkg/routes/api/v1/info.go @@ -67,7 +67,7 @@ type openIDAuthInfo struct { } type identityAwareProxyAuthInfo struct { - Enabled bool `json:"enabled"` + Enabled bool `json:"enabled"` } type legalInfo struct { diff --git a/pkg/routes/api/v1/login.go b/pkg/routes/api/v1/login.go index 33898855f..bf554a36f 100644 --- a/pkg/routes/api/v1/login.go +++ b/pkg/routes/api/v1/login.go @@ -99,7 +99,7 @@ func RenewToken(c echo.Context) (err error) { newToken, err := auth.RenewToken(s, c) if err != nil { _ = s.Rollback() - return handler.HandleHTTPError(err, c) + return handler.HandleHTTPError(err, c) } if err := s.Commit(); err != nil { diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index 34eacccd9..8de3381e2 100644 --- a/pkg/swagger/docs.go +++ b/pkg/swagger/docs.go @@ -32,16 +32,11 @@ var doc = `{ "host": "{{.Host}}", "basePath": "{{.BasePath}}", "paths": { - "/auth/identityawareproxy/login": { - "post": { - "security": [ - { - "JWTKeyAuth": [] - } - ], - "description": "After a redirect from the OpenID Connect provider to the frontend has been made with the authentication ` + "`" + `code` + "`" + `, this endpoint can be used to obtain a jwt token for that user and thus log them in.", + "/auth/identityawareproxy/token": { + "get": { + "description": "Generates a short-lived JWT based on the identity from the identity-aware proxy in order to provide the front-end with user id and username info", "consumes": [ - "application/json" + "N/A" ], "produces": [ "application/json" @@ -49,7 +44,7 @@ var doc = `{ "tags": [ "auth" ], - "summary": "Authenticate a user with OpenID Connect", + "summary": "Authenticate a user from the Identity-Aware Proxy", "responses": { "200": { "description": "OK", @@ -8631,9 +8626,6 @@ var doc = `{ "properties": { "enabled": { "type": "boolean" - }, - "name": { - "type": "string" } } }, diff --git a/pkg/swagger/swagger.json b/pkg/swagger/swagger.json index 64ad5d212..7225e0f6e 100644 --- a/pkg/swagger/swagger.json +++ b/pkg/swagger/swagger.json @@ -15,16 +15,11 @@ }, "basePath": "/api/v1", "paths": { - "/auth/identityawareproxy/login": { - "post": { - "security": [ - { - "JWTKeyAuth": [] - } - ], - "description": "After a redirect from the OpenID Connect provider to the frontend has been made with the authentication `code`, this endpoint can be used to obtain a jwt token for that user and thus log them in.", + "/auth/identityawareproxy/token": { + "get": { + "description": "Generates a short-lived JWT based on the identity from the identity-aware proxy in order to provide the front-end with user id and username info", "consumes": [ - "application/json" + "N/A" ], "produces": [ "application/json" @@ -32,7 +27,7 @@ "tags": [ "auth" ], - "summary": "Authenticate a user with OpenID Connect", + "summary": "Authenticate a user from the Identity-Aware Proxy", "responses": { "200": { "description": "OK", @@ -8614,9 +8609,6 @@ "properties": { "enabled": { "type": "boolean" - }, - "name": { - "type": "string" } } }, diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index f6d8afe33..aee8a7bc3 100644 --- a/pkg/swagger/swagger.yaml +++ b/pkg/swagger/swagger.yaml @@ -1210,8 +1210,6 @@ definitions: properties: enabled: type: boolean - name: - type: string type: object v1.legalInfo: properties: @@ -1338,11 +1336,11 @@ paths: summary: User Avatar tags: - user - /auth/identityawareproxy/login: - post: + /auth/identityawareproxy/token: + get: consumes: - - application/json - description: After a redirect from the OpenID Connect provider to the frontend has been made with the authentication `code`, this endpoint can be used to obtain a jwt token for that user and thus log them in. + - N/A + description: Generates a short-lived JWT based on the identity from the identity-aware proxy in order to provide the front-end with user id and username info produces: - application/json responses: @@ -1354,9 +1352,7 @@ paths: description: Internal error schema: $ref: '#/definitions/models.Message' - security: - - JWTKeyAuth: [] - summary: Authenticate a user with OpenID Connect + summary: Authenticate a user from the Identity-Aware Proxy tags: - auth /auth/openid/{provider}/callback: -- 2.40.1 From 93dee49b25a8a22fb71f598eb717093135abe602 Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Mon, 28 Dec 2020 16:30:00 -0500 Subject: [PATCH 3/9] Fix golint errors --- .golangci.yml | 8 ++++++++ pkg/config/config.go | 2 +- pkg/modules/auth/auth.go | 9 +++------ pkg/modules/auth/auth_test.go | 1 + pkg/modules/auth/identityawareproxy/error.go | 6 +++--- .../auth/identityawareproxy/identityawareproxy.go | 6 +++--- pkg/modules/auth/identityawareproxy/middleware.go | 6 +++--- 7 files changed, 22 insertions(+), 16 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 967ba5d1a..cb307aabe 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -80,3 +80,11 @@ issues: - text: "Missed string" linters: - goheader + - path: pkg/modules/auth/identityawareproxy/middleware_test.go + text: "G101:" # We don't care about hardcoded credentials in this test + linters: + - gosec + - path: pkg/modules/auth/auth.go + text: "stutters" + linters: + - golint diff --git a/pkg/config/config.go b/pkg/config/config.go index 2dfd436c5..66394ac3a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -60,7 +60,7 @@ const ( AuthOpenIDRedirectURL Key = `auth.openid.redirecturl` AuthOpenIDProviders Key = `auth.openid.providers` AuthIdentityAwareProxyEnabled Key = `auth.identityawareproxy.enabled` - AuthIdentityAwareProxyJwksUri Key = `auth.identityawareproxy.jwksuri` + AuthIdentityAwareProxyJwksURI Key = `auth.identityawareproxy.jwksuri` AuthIdentityAwareProxyJwtHeader Key = `auth.identityawareproxy.jwtheader` LegalImprintURL Key = `legal.imprinturl` diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index 139544382..fd03edffe 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -153,7 +153,7 @@ func GetAuthFromClaims(c echo.Context) (a web.Auth, err error) { return getLinkShareFromClaims(claims) } if claims.Type == AuthTypeUser { - return getUserFromClaims(claims) + return getUserFromClaims(claims), nil } if authProvider, ok := authProviders[claims.Type]; ok { return authProvider.GetWebAuth(c, claims) @@ -178,7 +178,7 @@ func getLinkShareFromClaims(claims *AuthClaims) (share *models.LinkSharing, err } // getUserFromClaims Returns a new user from jwt claims -func getUserFromClaims(claims *AuthClaims) (u *user.User, err error) { +func getUserFromClaims(claims *AuthClaims) (u *user.User) { u = &user.User{ ID: claims.UserID, Email: claims.UserEmail, @@ -223,10 +223,7 @@ func RenewToken(s *xorm.Session, c echo.Context) (token string, err error) { return NewLinkShareJWTAuthtoken(share) } if claims.Type == AuthTypeUser { - oldUser, err := getUserFromClaims(claims) - if err != nil { - return "", err - } + oldUser := getUserFromClaims(claims) u, err := user.GetUserWithEmail(s, &user.User{ID: oldUser.ID}) if err != nil { return "", err diff --git a/pkg/modules/auth/auth_test.go b/pkg/modules/auth/auth_test.go index 40dd112ce..0df33f020 100644 --- a/pkg/modules/auth/auth_test.go +++ b/pkg/modules/auth/auth_test.go @@ -32,6 +32,7 @@ func TestGetOrCreateUser(t *testing.T) { u, err := GetOrCreateUserFromExternalAuth(s, "https://some.issuer", "12345", "test@example.com", "", "someUserWhoDoesNotExistYet") assert.NoError(t, err) err = s.Commit() + assert.NoError(t, err) db.AssertExists(t, "users", map[string]interface{}{ "id": u.ID, diff --git a/pkg/modules/auth/identityawareproxy/error.go b/pkg/modules/auth/identityawareproxy/error.go index 8febb04bb..2b943b44c 100644 --- a/pkg/modules/auth/identityawareproxy/error.go +++ b/pkg/modules/auth/identityawareproxy/error.go @@ -48,7 +48,7 @@ func (err ErrIAPTokenMissing) HTTPError() web.HTTPError { // ErrIAPPublicKeysetMissing represents a "IAPPublicKeysetMissing" kind of error. type ErrIAPPublicKeysetMissing struct { - Url string + URL string } // IsErrIAPPublicKeysetMissing checks if an error is a ErrIAPPublicKeysetMissing. @@ -58,7 +58,7 @@ func IsErrIAPPublicKeysetMissing(err error) bool { } func (err ErrIAPPublicKeysetMissing) Error() string { - return fmt.Sprintf("Failed to retrive the identity-aware proxy's signing public key at URL: %s", err.Url) + return fmt.Sprintf("Failed to retrieve the identity-aware proxy's signing public key at URL: %s", err.URL) } // ErrorCodeIAPPublicKeysetMissing holds the unique world-error code of this error @@ -66,7 +66,7 @@ const ErrorCodeIAPPublicKeysetMissing = 12002 // HTTPError holds the http error description func (err ErrIAPPublicKeysetMissing) HTTPError() web.HTTPError { - return web.HTTPError{HTTPCode: http.StatusServiceUnavailable, Code: ErrorCodeIAPPublicKeysetMissing, Message: "Failed to retrive the identity-aware proxy's signing public keys."} + return web.HTTPError{HTTPCode: http.StatusServiceUnavailable, Code: ErrorCodeIAPPublicKeysetMissing, Message: "Failed to retrieve the identity-aware proxy's signing public keys."} } // ErrIAPUserFrontendMismatch represents a "IAPUserDoesNotMatchFrontendUser" kind of error. diff --git a/pkg/modules/auth/identityawareproxy/identityawareproxy.go b/pkg/modules/auth/identityawareproxy/identityawareproxy.go index ba89eb298..04ef02b3c 100644 --- a/pkg/modules/auth/identityawareproxy/identityawareproxy.go +++ b/pkg/modules/auth/identityawareproxy/identityawareproxy.go @@ -52,7 +52,7 @@ func init() { // These are intentionally short lived because they can be regenerated at // any time from the IAP authn information. They are not related to // session length and are only used to provide user info to the frontend -// and a hint to auth.go to retreive auth data from the IAP. +// and a hint to auth.go to retrieve auth data from the IAP. func NewIAPUserJWTAuthtoken(u *user.User) (token string, err error) { // Set claims claims := &auth.AuthClaims{ @@ -141,11 +141,11 @@ func (c *IAPClaims) Valid() error { // Validate that expiresAt and issuedAt are set and valid (with up to 1 minute of skew) now := TimeFunc() skew := time.Minute - if c.VerifyExpiresAt(now.Add(-skew).Unix(), true) == false { + if !c.VerifyExpiresAt(now.Add(-skew).Unix(), true) { delta := now.Sub(time.Unix(c.ExpiresAt, 0)) return fmt.Errorf("token is expired by %v", delta) } - if c.VerifyIssuedAt(now.Add(skew).Unix(), true) == false { + if !c.VerifyIssuedAt(now.Add(skew).Unix(), true) { return fmt.Errorf("token used before issued") } diff --git a/pkg/modules/auth/identityawareproxy/middleware.go b/pkg/modules/auth/identityawareproxy/middleware.go index a700a296e..e4ec0838d 100644 --- a/pkg/modules/auth/identityawareproxy/middleware.go +++ b/pkg/modules/auth/identityawareproxy/middleware.go @@ -55,10 +55,10 @@ func (cache *iapCache) GetKeyset() (*jwk.Set, error) { } // Fetch the public key(s) from the identity-aware proxy - keyset, err := jwk.FetchHTTP(config.AuthIdentityAwareProxyJwksUri.GetString()) + keyset, err := jwk.FetchHTTP(config.AuthIdentityAwareProxyJwksURI.GetString()) if err != nil { - log.Error("Failed to retrive the identity-aware proxy's signing public key at URL %s: %v", config.AuthIdentityAwareProxyJwksUri.GetString(), err) - return nil, ErrIAPPublicKeysetMissing{Url: config.AuthIdentityAwareProxyJwksUri.GetString()} + log.Error("Failed to retrieve the identity-aware proxy's signing public key at URL %s: %v", config.AuthIdentityAwareProxyJwksURI.GetString(), err) + return nil, ErrIAPPublicKeysetMissing{URL: config.AuthIdentityAwareProxyJwksURI.GetString()} } cache.keyset = keyset return cache.keyset, nil -- 2.40.1 From 440e0e2b2747efe723da4e08fffe028215d02f6f Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Mon, 17 May 2021 15:34:19 -0400 Subject: [PATCH 4/9] Fix rebase errors --- pkg/modules/auth/auth.go | 2 +- pkg/modules/auth/openid/openid.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index fd03edffe..302b9f99e 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -216,7 +216,7 @@ func RenewToken(s *xorm.Session, c echo.Context) (token string, err error) { } share := &models.LinkSharing{} share.ID = oldShare.ID - err = share.ReadOne(s) + err = share.ReadOne(s, share) if err != nil { return "", err } diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 8ba382aea..b445b0c0b 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -30,6 +30,7 @@ import ( "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" + "code.vikunja.io/api/pkg/user" "github.com/coreos/go-oidc" "github.com/labstack/echo/v4" "golang.org/x/oauth2" -- 2.40.1 From 911dffd8b3160a69fdfb4bfd2d13c0464e7ec962 Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Mon, 17 May 2021 23:09:32 -0400 Subject: [PATCH 5/9] Make IAP middleware an optional follow-up to the JWT middleware --- pkg/initialize/init.go | 4 + pkg/modules/auth/auth.go | 32 +++---- pkg/modules/auth/identityawareproxy/error.go | 8 +- .../identityawareproxy/identityawareproxy.go | 95 +++++++++---------- .../identityawareproxy_test.go | 8 +- .../auth/identityawareproxy/middleware.go | 27 +++++- .../identityawareproxy/middleware_test.go | 8 +- pkg/modules/auth/middleware.go | 80 ++++++++++++++++ pkg/routes/routes.go | 13 +-- pkg/swagger/swagger.yaml | 3 +- 10 files changed, 178 insertions(+), 100 deletions(-) create mode 100644 pkg/modules/auth/middleware.go diff --git a/pkg/initialize/init.go b/pkg/initialize/init.go index 32e067ac2..87fb940f7 100644 --- a/pkg/initialize/init.go +++ b/pkg/initialize/init.go @@ -27,6 +27,7 @@ import ( "code.vikunja.io/api/pkg/mail" "code.vikunja.io/api/pkg/migration" "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth/identityawareproxy" "code.vikunja.io/api/pkg/modules/keyvalue" migrator "code.vikunja.io/api/pkg/modules/migration" "code.vikunja.io/api/pkg/notifications" @@ -95,6 +96,9 @@ func FullInit() { models.RegisterReminderCron() models.RegisterOverdueReminderCron() + // Setup + identityawareproxy.Init() + // Start processing events go func() { models.RegisterListeners() diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index 302b9f99e..779aed133 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -29,7 +29,6 @@ import ( "github.com/dgrijalva/jwt-go" petname "github.com/dustinkirkland/golang-petname" "github.com/labstack/echo/v4" - "github.com/labstack/echo/v4/middleware" ) // These are all valid auth types @@ -42,7 +41,8 @@ const ( AuthTypeIAPUser ) -const authTokenContextKey string = "authToken" +// Key used to store authClaims +const AuthClaimsContextKey string = "authClaims" // Token represents an authentification token in signed string form type Token struct { @@ -78,7 +78,8 @@ type AuthClaims struct { // In these cases, AuthClaims may contain hints to the user identity, // but an outside source is the final source-of-truth for auth (e.g. Identity-Aware Proxy auth) type AuthProvider interface { - GetWebAuth(echo.Context, *AuthClaims) (web.Auth, error) + GetUser(echo.Context, *AuthClaims) (*user.User, error) + RenewToken(echo.Context, *AuthClaims) (string, error) } var authProviders = map[AuthType]AuthProvider{} @@ -147,8 +148,7 @@ func NewLinkShareJWTAuthtoken(share *models.LinkSharing) (token string, err erro // GetAuthFromClaims returns a web.Auth object from jwt claims or from an // alternative authProvider func GetAuthFromClaims(c echo.Context) (a web.Auth, err error) { - jwtinf := c.Get(authTokenContextKey).(*jwt.Token) - claims := jwtinf.Claims.(*AuthClaims) + claims := c.Get(AuthClaimsContextKey).(*AuthClaims) if claims.Type == AuthTypeLinkShare && config.ServiceEnableLinkSharing.GetBool() { return getLinkShareFromClaims(claims) } @@ -156,7 +156,7 @@ func GetAuthFromClaims(c echo.Context) (a web.Auth, err error) { return getUserFromClaims(claims), nil } if authProvider, ok := authProviders[claims.Type]; ok { - return authProvider.GetWebAuth(c, claims) + return authProvider.GetUser(c, claims) } return nil, echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) } @@ -206,8 +206,7 @@ func GetCurrentUser(c echo.Context) (u *user.User, err error) { // Generates a new jwt token for the types AuthTypeLinkShare and AuthTypeUser func RenewToken(s *xorm.Session, c echo.Context) (token string, err error) { - jwtinf := c.Get(authTokenContextKey).(*jwt.Token) - claims := jwtinf.Claims.(*AuthClaims) + claims := c.Get(AuthClaimsContextKey).(*AuthClaims) if claims.Type == AuthTypeLinkShare { oldShare, err := getLinkShareFromClaims(claims) @@ -230,17 +229,14 @@ func RenewToken(s *xorm.Session, c echo.Context) (token string, err error) { } return NewUserJWTAuthtoken(u) } - return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) -} - -// GetJWTConfig returns the config for the default JWT middleware -func GetJWTConfig() middleware.JWTConfig { - return middleware.JWTConfig{ - SigningKey: []byte(config.ServiceJWTSecret.GetString()), - SigningMethod: middleware.AlgorithmHS256, - ContextKey: authTokenContextKey, - Claims: &AuthClaims{}, + if authProvider, ok := authProviders[claims.Type]; ok { + token, err := authProvider.RenewToken(c, claims) + if err != nil { + return "", err + } + return token, nil } + return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) } // GetOrCreateUserFromExternalAuth returns a user after finding or creating a matching user for the provided details diff --git a/pkg/modules/auth/identityawareproxy/error.go b/pkg/modules/auth/identityawareproxy/error.go index 2b943b44c..1bc36ba58 100644 --- a/pkg/modules/auth/identityawareproxy/error.go +++ b/pkg/modules/auth/identityawareproxy/error.go @@ -1,17 +1,17 @@ // Vikunja is a to-do list application to facilitate your life. -// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. // // This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by +// it under the terms of the GNU Affero General Public Licensee as published by // the Free Software Foundation, either version 3 of the License, or // (at your option) any later version. // // This program is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. +// GNU Affero General Public Licensee for more details. // -// You should have received a copy of the GNU General Public License +// You should have received a copy of the GNU Affero General Public Licensee // along with this program. If not, see . package identityawareproxy diff --git a/pkg/modules/auth/identityawareproxy/identityawareproxy.go b/pkg/modules/auth/identityawareproxy/identityawareproxy.go index 04ef02b3c..5c06a7688 100644 --- a/pkg/modules/auth/identityawareproxy/identityawareproxy.go +++ b/pkg/modules/auth/identityawareproxy/identityawareproxy.go @@ -1,17 +1,17 @@ // Vikunja is a to-do list application to facilitate your life. -// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. // // This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by +// it under the terms of the GNU Affero General Public Licensee as published by // the Free Software Foundation, either version 3 of the License, or // (at your option) any later version. // // This program is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. +// GNU Affero General Public Licensee for more details. // -// You should have received a copy of the GNU General Public License +// You should have received a copy of the GNU Affero General Public Licensee // along with this program. If not, see . package identityawareproxy @@ -25,7 +25,6 @@ import ( "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/user" - "code.vikunja.io/web" "github.com/dgrijalva/jwt-go" "github.com/labstack/echo/v4" ) @@ -44,8 +43,9 @@ type IAPClaims struct { // Auth provider used to allow auth to get a web.Auth from the IAP provided identity type IAPAuthProvider struct{} -func init() { +func Init() { auth.RegisterAuthProvider(auth.AuthTypeIAPUser, IAPAuthProvider{}) + auth.RegisterAuthMiddleware(auth.AuthTypeIAPUser, Middleware()) } // NewIAPUserJWTAuthtoken generates and signes a new jwt token for a user @@ -55,7 +55,15 @@ func init() { // and a hint to auth.go to retrieve auth data from the IAP. func NewIAPUserJWTAuthtoken(u *user.User) (token string, err error) { // Set claims - claims := &auth.AuthClaims{ + claims := newIAPUserJWTAuthClaims(u) + t := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + + // Generate encoded token and send it as response. + return t.SignedString([]byte(config.ServiceJWTSecret.GetString())) +} + +func newIAPUserJWTAuthClaims(u *user.User) (claims *auth.AuthClaims) { + return &auth.AuthClaims{ Type: auth.AuthTypeIAPUser, UserID: u.ID, UserUsername: u.Username, @@ -65,57 +73,14 @@ func NewIAPUserJWTAuthtoken(u *user.User) (token string, err error) { ExpiresAt: time.Now().Add(time.Minute * 5).Unix(), }, } - - t := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) - - // Generate encoded token and send it as response. - return t.SignedString([]byte(config.ServiceJWTSecret.GetString())) } -// Token generates a local, short-lived JWT based on the identity from the identity-aware proxy. -// See also the docs for NewIAPUserJWTAuthtoken -// @Summary Authenticate a user from the Identity-Aware Proxy -// @Description Generates a short-lived JWT based on the identity from the identity-aware proxy in order to provide the front-end with user id and username info -// @tags auth -// @Accept N/A -// @Produce json -// @Success 200 {object} auth.Token -// @Failure 500 {object} models.Message "Internal error" -// @Router /auth/identityawareproxy/token [get] -func GetToken(c echo.Context) error { - cl := c.Get(IAPClaimsContextKey).(*IAPClaims) - +func userForIAPClaims(cl *IAPClaims) (u *user.User, err error) { s := db.NewSession() defer s.Close() // Check if we have seen this user before - u, err := auth.GetOrCreateUserFromExternalAuth(s, cl.Issuer, cl.Subject, cl.Email, cl.Name, cl.PreferredUsername) - if err != nil { - _ = s.Rollback() - return err - } - - err = s.Commit() - if err != nil { - return err - } - - // Create token - userToken, err := NewIAPUserJWTAuthtoken(u) - if err != nil { - return err - } - return auth.NewTokenResponse(userToken, c) -} - -// Get a web.Auth object from the identity that the IAP provides -func (p IAPAuthProvider) GetWebAuth(c echo.Context, authClaims *auth.AuthClaims) (web.Auth, error) { - s := db.NewSession() - defer s.Close() - - // Get the user from the IAP identity - cl := c.Get(IAPClaimsContextKey).(*IAPClaims) - u, err := auth.GetOrCreateUserFromExternalAuth(s, cl.Issuer, cl.Subject, cl.Email, cl.Name, cl.PreferredUsername) + u, err = auth.GetOrCreateUserFromExternalAuth(s, cl.Issuer, cl.Subject, cl.Email, cl.Name, cl.PreferredUsername) if err != nil { _ = s.Rollback() return nil, err @@ -126,6 +91,32 @@ func (p IAPAuthProvider) GetWebAuth(c echo.Context, authClaims *auth.AuthClaims) return nil, err } + return u, nil +} + +func (p IAPAuthProvider) RenewToken(c echo.Context, authClaims *auth.AuthClaims) (string, error) { + // Get user + u, err := p.GetUser(c, authClaims) + if err != nil { + return "", nil + } + + // Create token + return NewIAPUserJWTAuthtoken(u) +} + +// Get a web.Auth object from the identity that the IAP provides +func (p IAPAuthProvider) GetUser(c echo.Context, authClaims *auth.AuthClaims) (*user.User, error) { + s := db.NewSession() + defer s.Close() + + // Get the user from the IAP identity + cl := c.Get(IAPClaimsContextKey).(*IAPClaims) + u, err := userForIAPClaims(cl) + if err != nil { + return nil, err + } + // Sanity check that the user the frontend thinks it has (the authClaims from the JWT it passed in) // is the same as the user provided by the IAP. if authClaims.UserID != u.ID { diff --git a/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go b/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go index 55a28ce4c..c438217f4 100644 --- a/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go +++ b/pkg/modules/auth/identityawareproxy/identityawareproxy_test.go @@ -1,17 +1,17 @@ // Vikunja is a to-do list application to facilitate your life. -// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. // // This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by +// it under the terms of the GNU Affero General Public Licensee as published by // the Free Software Foundation, either version 3 of the License, or // (at your option) any later version. // // This program is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. +// GNU Affero General Public Licensee for more details. // -// You should have received a copy of the GNU General Public License +// You should have received a copy of the GNU Affero General Public Licensee // along with this program. If not, see . package identityawareproxy diff --git a/pkg/modules/auth/identityawareproxy/middleware.go b/pkg/modules/auth/identityawareproxy/middleware.go index e4ec0838d..701c77648 100644 --- a/pkg/modules/auth/identityawareproxy/middleware.go +++ b/pkg/modules/auth/identityawareproxy/middleware.go @@ -1,17 +1,17 @@ // Vikunja is a to-do list application to facilitate your life. -// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. // // This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by +// it under the terms of the GNU Affero General Public Licensee as published by // the Free Software Foundation, either version 3 of the License, or // (at your option) any later version. // // This program is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. +// GNU Affero General Public Licensee for more details. // -// You should have received a copy of the GNU General Public License +// You should have received a copy of the GNU Affero General Public Licensee // along with this program. If not, see . package identityawareproxy @@ -23,6 +23,7 @@ import ( "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/log" + "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/web/handler" "github.com/dgrijalva/jwt-go" "github.com/labstack/echo/v4" @@ -70,6 +71,12 @@ func Middleware() echo.MiddlewareFunc { cache := &iapCache{} return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { + // Skip if IAP is not enabled + if !config.AuthIdentityAwareProxyEnabled.GetBool() { + return next(c) + } + + // Get and validate claims from the IAP token := c.Request().Header.Get(config.AuthIdentityAwareProxyJwtHeader.GetString()) if token == "" { err := ErrIAPTokenMissing{Header: config.AuthIdentityAwareProxyJwtHeader.GetString()} @@ -85,13 +92,23 @@ func Middleware() echo.MiddlewareFunc { if err != nil { return handler.HandleHTTPError(err, c) } - c.Set(IAPClaimsContextKey, cl) + + // Generate auth.AuthClaims from the IAP identity + user, err := userForIAPClaims(cl) + if err != nil { + return handler.HandleHTTPError(err, c) + } + authClaims := newIAPUserJWTAuthClaims(user) + c.Set(auth.AuthClaimsContextKey, authClaims) return next(c) } } } +// The authMiddleware generates and stores internal auth based on +// those claims. This overwrites any auth from the JWT middleware + func parseAndValidateJwt(token string, keyset *jwk.Set) (*IAPClaims, error) { // Parse the jwt from the identity-aware proxy using the correct key tken, err := jwt.ParseWithClaims(token, &IAPClaims{}, func(unvalidatedToken *jwt.Token) (interface{}, error) { diff --git a/pkg/modules/auth/identityawareproxy/middleware_test.go b/pkg/modules/auth/identityawareproxy/middleware_test.go index 1ec3062bf..5b9477a86 100644 --- a/pkg/modules/auth/identityawareproxy/middleware_test.go +++ b/pkg/modules/auth/identityawareproxy/middleware_test.go @@ -1,17 +1,17 @@ // Vikunja is a to-do list application to facilitate your life. -// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. // // This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by +// it under the terms of the GNU Affero General Public Licensee as published by // the Free Software Foundation, either version 3 of the License, or // (at your option) any later version. // // This program is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. +// GNU Affero General Public Licensee for more details. // -// You should have received a copy of the GNU General Public License +// You should have received a copy of the GNU Affero General Public Licensee // along with this program. If not, see . package identityawareproxy diff --git a/pkg/modules/auth/middleware.go b/pkg/modules/auth/middleware.go new file mode 100644 index 000000000..cd24aa0c8 --- /dev/null +++ b/pkg/modules/auth/middleware.go @@ -0,0 +1,80 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public Licensee as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public Licensee for more details. +// +// You should have received a copy of the GNU Affero General Public Licensee +// along with this program. If not, see . + +package auth + +import ( + "code.vikunja.io/api/pkg/config" + "github.com/dgrijalva/jwt-go" + "github.com/labstack/echo/v4" + "github.com/labstack/echo/v4/middleware" +) + +// Resolves circular dependencies of auth -> IAP -> auth +var authMiddlewares = map[AuthType]echo.MiddlewareFunc{} + +func RegisterAuthMiddleware(t AuthType, f echo.MiddlewareFunc) { + authMiddlewares[t] = f +} + +const authTokenJWTContextKey = "jwtToken" + +// GetJWTConfig returns the config for the default JWT middleware +func GetJWTConfig() middleware.JWTConfig { + return middleware.JWTConfig{ + SigningKey: []byte(config.ServiceJWTSecret.GetString()), + SigningMethod: middleware.AlgorithmHS256, + ContextKey: authTokenJWTContextKey, + Claims: &AuthClaims{}, + } +} + +// The auth middleware uses the JWT middleware to parse and validate a JWT. +// If that does not succeed, it generates a JWT token from the identity-aware proxy +func Middleware() echo.MiddlewareFunc { + // Create a noop next function to let us run middlewares without jumping to the next + // one in the chain + noOpNext := func(_ echo.Context) error { + return nil + } + jwtMiddleware := middleware.JWTWithConfig(GetJWTConfig()) + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + // First attempt to get auth from a provided JWT + jwtErr := jwtMiddleware(noOpNext)(c) + if c.Get(authTokenJWTContextKey) != nil && jwtErr == nil { + // If it succeeded, use the authClaims from the JWT + // and continue in the middleware chain + jwtinf := c.Get(authTokenJWTContextKey).(*jwt.Token) + claims := jwtinf.Claims.(*AuthClaims) + c.Set(AuthClaimsContextKey, claims) + return next(c) + } + + // Otherwise, attempt to get auth from authMiddlewares + for _, authMiddleware := range authMiddlewares { + err := authMiddleware(noOpNext)(c) + if c.Get(AuthClaimsContextKey) != nil && err == nil { + // If it succeeded, continue in the middleware chain + return next(c) + } + } + + // Otherwise, return the original error from jwt middleware + return jwtErr + } + } +} diff --git a/pkg/routes/routes.go b/pkg/routes/routes.go index 3cdc88b31..9e0484e69 100644 --- a/pkg/routes/routes.go +++ b/pkg/routes/routes.go @@ -55,7 +55,6 @@ import ( "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" - "code.vikunja.io/api/pkg/modules/auth/identityawareproxy" "code.vikunja.io/api/pkg/modules/auth/openid" "code.vikunja.io/api/pkg/modules/background" backgroundHandler "code.vikunja.io/api/pkg/modules/background/handler" @@ -251,19 +250,9 @@ func registerAPIRoutes(a *echo.Group) { n.POST("/shares/:share/auth", apiv1.AuthenticateLinkShare) } - // Identity-Aware Proxy auth, requires the same iap middleware as authenticated routes - if config.AuthIdentityAwareProxyEnabled.GetBool() { - m := n.Group("") - m.Use(identityawareproxy.Middleware()) - m.GET("/auth/identityawareproxy/token", identityawareproxy.GetToken) - } - // ===== Routes with Authentication ===== // Authentification - a.Use(middleware.JWTWithConfig(auth.GetJWTConfig())) - if config.AuthIdentityAwareProxyEnabled.GetBool() { - a.Use(identityawareproxy.Middleware()) - } + a.Use(auth.Middleware()) // Rate limit setupRateLimit(a, config.RateLimitKind.GetString()) diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index aee8a7bc3..af8eb15d0 100644 --- a/pkg/swagger/swagger.yaml +++ b/pkg/swagger/swagger.yaml @@ -1340,7 +1340,8 @@ paths: get: consumes: - N/A - description: Generates a short-lived JWT based on the identity from the identity-aware proxy in order to provide the front-end with user id and username info + description: Generates a short-lived JWT based on the identity from the identity-aware + proxy in order to provide the front-end with user id and username info produces: - application/json responses: -- 2.40.1 From 575414b329e507131d22d039b69bc58bec19a24c Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Tue, 18 May 2021 18:25:29 -0400 Subject: [PATCH 6/9] No need to renew token for IAP --- pkg/modules/auth/auth.go | 7 +- .../identityawareproxy/identityawareproxy.go | 68 +++++-------------- 2 files changed, 18 insertions(+), 57 deletions(-) diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index 779aed133..f1ff31265 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -79,7 +79,6 @@ type AuthClaims struct { // but an outside source is the final source-of-truth for auth (e.g. Identity-Aware Proxy auth) type AuthProvider interface { GetUser(echo.Context, *AuthClaims) (*user.User, error) - RenewToken(echo.Context, *AuthClaims) (string, error) } var authProviders = map[AuthType]AuthProvider{} @@ -230,11 +229,7 @@ func RenewToken(s *xorm.Session, c echo.Context) (token string, err error) { return NewUserJWTAuthtoken(u) } if authProvider, ok := authProviders[claims.Type]; ok { - token, err := authProvider.RenewToken(c, claims) - if err != nil { - return "", err - } - return token, nil + return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "External auth types do not use JWT tokens."} } return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) } diff --git a/pkg/modules/auth/identityawareproxy/identityawareproxy.go b/pkg/modules/auth/identityawareproxy/identityawareproxy.go index 5c06a7688..3c1dac16a 100644 --- a/pkg/modules/auth/identityawareproxy/identityawareproxy.go +++ b/pkg/modules/auth/identityawareproxy/identityawareproxy.go @@ -48,33 +48,7 @@ func Init() { auth.RegisterAuthMiddleware(auth.AuthTypeIAPUser, Middleware()) } -// NewIAPUserJWTAuthtoken generates and signes a new jwt token for a user -// These are intentionally short lived because they can be regenerated at -// any time from the IAP authn information. They are not related to -// session length and are only used to provide user info to the frontend -// and a hint to auth.go to retrieve auth data from the IAP. -func NewIAPUserJWTAuthtoken(u *user.User) (token string, err error) { - // Set claims - claims := newIAPUserJWTAuthClaims(u) - t := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) - - // Generate encoded token and send it as response. - return t.SignedString([]byte(config.ServiceJWTSecret.GetString())) -} - -func newIAPUserJWTAuthClaims(u *user.User) (claims *auth.AuthClaims) { - return &auth.AuthClaims{ - Type: auth.AuthTypeIAPUser, - UserID: u.ID, - UserUsername: u.Username, - UserEmail: u.Email, - UserName: u.Name, - StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Add(time.Minute * 5).Unix(), - }, - } -} - +// Get or generate a new user for the claims made by the IAP func userForIAPClaims(cl *IAPClaims) (u *user.User, err error) { s := db.NewSession() defer s.Close() @@ -94,39 +68,31 @@ func userForIAPClaims(cl *IAPClaims) (u *user.User, err error) { return u, nil } -func (p IAPAuthProvider) RenewToken(c echo.Context, authClaims *auth.AuthClaims) (string, error) { - // Get user - u, err := p.GetUser(c, authClaims) - if err != nil { - return "", nil +// Create an AuthClaims object for a user dervived from an IAP identity +func newIAPUserJWTAuthClaims(u *user.User) (claims *auth.AuthClaims) { + return &auth.AuthClaims{ + Type: auth.AuthTypeIAPUser, + UserID: u.ID, + UserUsername: u.Username, + UserEmail: u.Email, + UserName: u.Name, } - - // Create token - return NewIAPUserJWTAuthtoken(u) } // Get a web.Auth object from the identity that the IAP provides func (p IAPAuthProvider) GetUser(c echo.Context, authClaims *auth.AuthClaims) (*user.User, error) { - s := db.NewSession() - defer s.Close() - - // Get the user from the IAP identity - cl := c.Get(IAPClaimsContextKey).(*IAPClaims) - u, err := userForIAPClaims(cl) - if err != nil { - return nil, err + // The IAP middleware already checked and created a user if needed, no need to regenerate them + // Just use the authClaims provided by the middleware + u = &user.User{ + ID: authClaims.UserID, + Email: authClaims.UserEmail, + Username: authClaims.UserUsername, + Name: authClaims.UserName, } - - // Sanity check that the user the frontend thinks it has (the authClaims from the JWT it passed in) - // is the same as the user provided by the IAP. - if authClaims.UserID != u.ID { - return nil, ErrIAPUserFrontendMismatch{} - } - return u, nil } -// Validates the claims in the jwt +// Validates the claims in the IAP jwt // Matches the jwt-go Claims interface func (c *IAPClaims) Valid() error { // Validate that expiresAt and issuedAt are set and valid (with up to 1 minute of skew) -- 2.40.1 From 213a7db6613621fc94c7c1ab5407650a13ad8e32 Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Tue, 18 May 2021 18:30:00 -0400 Subject: [PATCH 7/9] fix --- pkg/modules/auth/auth.go | 4 ++-- pkg/modules/auth/identityawareproxy/identityawareproxy.go | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index f1ff31265..09286daf2 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -228,8 +228,8 @@ func RenewToken(s *xorm.Session, c echo.Context) (token string, err error) { } return NewUserJWTAuthtoken(u) } - if authProvider, ok := authProviders[claims.Type]; ok { - return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "External auth types do not use JWT tokens."} + if _, ok := authProviders[claims.Type]; ok { + return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "External auth types do not use JWT tokens."}) } return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) } diff --git a/pkg/modules/auth/identityawareproxy/identityawareproxy.go b/pkg/modules/auth/identityawareproxy/identityawareproxy.go index 3c1dac16a..de6ba50c0 100644 --- a/pkg/modules/auth/identityawareproxy/identityawareproxy.go +++ b/pkg/modules/auth/identityawareproxy/identityawareproxy.go @@ -21,8 +21,6 @@ import ( "time" "code.vikunja.io/api/pkg/db" - - "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/user" "github.com/dgrijalva/jwt-go" @@ -83,7 +81,7 @@ func newIAPUserJWTAuthClaims(u *user.User) (claims *auth.AuthClaims) { func (p IAPAuthProvider) GetUser(c echo.Context, authClaims *auth.AuthClaims) (*user.User, error) { // The IAP middleware already checked and created a user if needed, no need to regenerate them // Just use the authClaims provided by the middleware - u = &user.User{ + u := &user.User{ ID: authClaims.UserID, Email: authClaims.UserEmail, Username: authClaims.UserUsername, -- 2.40.1 From 717c3bcc0be7144a4eedc3ce024e6cf65aaa1841 Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Tue, 18 May 2021 18:42:44 -0400 Subject: [PATCH 8/9] Clean up swagger --- pkg/modules/auth/middleware.go | 2 +- pkg/swagger/docs.go | 29 ----------------------------- pkg/swagger/swagger.json | 29 ----------------------------- pkg/swagger/swagger.yaml | 20 -------------------- 4 files changed, 1 insertion(+), 79 deletions(-) diff --git a/pkg/modules/auth/middleware.go b/pkg/modules/auth/middleware.go index cd24aa0c8..2091e4daf 100644 --- a/pkg/modules/auth/middleware.go +++ b/pkg/modules/auth/middleware.go @@ -72,7 +72,7 @@ func Middleware() echo.MiddlewareFunc { return next(c) } } - + // Otherwise, return the original error from jwt middleware return jwtErr } diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index 8de3381e2..5e954e86e 100644 --- a/pkg/swagger/docs.go +++ b/pkg/swagger/docs.go @@ -32,35 +32,6 @@ var doc = `{ "host": "{{.Host}}", "basePath": "{{.BasePath}}", "paths": { - "/auth/identityawareproxy/token": { - "get": { - "description": "Generates a short-lived JWT based on the identity from the identity-aware proxy in order to provide the front-end with user id and username info", - "consumes": [ - "N/A" - ], - "produces": [ - "application/json" - ], - "tags": [ - "auth" - ], - "summary": "Authenticate a user from the Identity-Aware Proxy", - "responses": { - "200": { - "description": "OK", - "schema": { - "$ref": "#/definitions/auth.Token" - } - }, - "500": { - "description": "Internal error", - "schema": { - "$ref": "#/definitions/models.Message" - } - } - } - } - }, "/auth/openid/{provider}/callback": { "post": { "security": [ diff --git a/pkg/swagger/swagger.json b/pkg/swagger/swagger.json index 7225e0f6e..7e3593b3d 100644 --- a/pkg/swagger/swagger.json +++ b/pkg/swagger/swagger.json @@ -15,35 +15,6 @@ }, "basePath": "/api/v1", "paths": { - "/auth/identityawareproxy/token": { - "get": { - "description": "Generates a short-lived JWT based on the identity from the identity-aware proxy in order to provide the front-end with user id and username info", - "consumes": [ - "N/A" - ], - "produces": [ - "application/json" - ], - "tags": [ - "auth" - ], - "summary": "Authenticate a user from the Identity-Aware Proxy", - "responses": { - "200": { - "description": "OK", - "schema": { - "$ref": "#/definitions/auth.Token" - } - }, - "500": { - "description": "Internal error", - "schema": { - "$ref": "#/definitions/models.Message" - } - } - } - } - }, "/auth/openid/{provider}/callback": { "post": { "security": [ diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index af8eb15d0..6f7d0b26c 100644 --- a/pkg/swagger/swagger.yaml +++ b/pkg/swagger/swagger.yaml @@ -1336,26 +1336,6 @@ paths: summary: User Avatar tags: - user - /auth/identityawareproxy/token: - get: - consumes: - - N/A - description: Generates a short-lived JWT based on the identity from the identity-aware - proxy in order to provide the front-end with user id and username info - produces: - - application/json - responses: - "200": - description: OK - schema: - $ref: '#/definitions/auth.Token' - "500": - description: Internal error - schema: - $ref: '#/definitions/models.Message' - summary: Authenticate a user from the Identity-Aware Proxy - tags: - - auth /auth/openid/{provider}/callback: post: consumes: -- 2.40.1 From c9ab22cec7cd7cdf057faa9bb6b70175bc60fd0a Mon Sep 17 00:00:00 2001 From: branchmispredictor Date: Tue, 18 May 2021 18:50:47 -0400 Subject: [PATCH 9/9] move to keyvalue --- .../auth/identityawareproxy/middleware.go | 42 ++++++++----------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/pkg/modules/auth/identityawareproxy/middleware.go b/pkg/modules/auth/identityawareproxy/middleware.go index 701c77648..10ac0b8b5 100644 --- a/pkg/modules/auth/identityawareproxy/middleware.go +++ b/pkg/modules/auth/identityawareproxy/middleware.go @@ -18,12 +18,12 @@ package identityawareproxy import ( "fmt" - "sync" "time" "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/modules/auth" + "code.vikunja.io/api/pkg/modules/keyvalue" "code.vikunja.io/web/handler" "github.com/dgrijalva/jwt-go" "github.com/labstack/echo/v4" @@ -35,40 +35,32 @@ import ( var TimeFunc = time.Now // Caches the public keys of the identity-aware proxy used to validate the auth data it sends -type iapCache struct { - keyset *jwk.Set - mutex sync.Mutex -} +const iapCacheKey = "iapcache_keyset" // GetKeyset returns the cached public keys from the identity-aware proxy // or fetches them for the first time. -func (cache *iapCache) GetKeyset() (*jwk.Set, error) { - if cache.keyset != nil { - return cache.keyset, nil +func GetKeyset() (keyset *jwk.Set, err error) { + k, exists, err := keyvalue.Get(iapCacheKey) + if !exists { + // Fetch the public key(s) from the identity-aware proxy + keyset, err = jwk.FetchHTTP(config.AuthIdentityAwareProxyJwksURI.GetString()) + if err != nil { + log.Error("Failed to retrieve the identity-aware proxy's signing public key at URL %s: %v", config.AuthIdentityAwareProxyJwksURI.GetString(), err) + return nil, ErrIAPPublicKeysetMissing{URL: config.AuthIdentityAwareProxyJwksURI.GetString()} + } + keyvalue.Put(iapCacheKey, keyset) } - cache.mutex.Lock() - defer cache.mutex.Unlock() - - // Check that another thread has not fetched the keyset - if cache.keyset != nil { - return cache.keyset, nil + if k != nil { + return k.(*jwk.Set), nil } - - // Fetch the public key(s) from the identity-aware proxy - keyset, err := jwk.FetchHTTP(config.AuthIdentityAwareProxyJwksURI.GetString()) - if err != nil { - log.Error("Failed to retrieve the identity-aware proxy's signing public key at URL %s: %v", config.AuthIdentityAwareProxyJwksURI.GetString(), err) - return nil, ErrIAPPublicKeysetMissing{URL: config.AuthIdentityAwareProxyJwksURI.GetString()} - } - cache.keyset = keyset - return cache.keyset, nil + + return } // The identity-aware proxy authentication middleware parses and validates the // JWT provided by the IAP func Middleware() echo.MiddlewareFunc { - cache := &iapCache{} return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { // Skip if IAP is not enabled @@ -83,7 +75,7 @@ func Middleware() echo.MiddlewareFunc { return handler.HandleHTTPError(err, c) } - keyset, err := cache.GetKeyset() + keyset, err := GetKeyset() if err != nil { return handler.HandleHTTPError(err, c) } -- 2.40.1