From c4104488e503455ceb2772d13d02c49ad0c61e91 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Thu, 18 Apr 2019 18:21:26 +0200 Subject: [PATCH] More strict ext-auth protocols, remove jwt cookie + small fixes --- api/system/spec.json | 16 ++++++ api/system/spec/auth.json | 16 ++++++ docs/Authentication.md | 14 +++--- docs/system/README.md | 15 ++++++ internal/auth/interfaces.go | 5 -- internal/auth/jwt.go | 19 ------- internal/settings/settings.go | 4 +- system/cli/settings.go | 6 ++- system/internal/service/auth.go | 29 +++++++++++ system/internal/service/auth_settings.go | 17 +++++++ system/rest/auth.go | 32 ++++++++---- system/rest/auth_external.go | 64 ++++++++++++++++++++---- system/rest/auth_internal.go | 7 ++- system/rest/handlers/auth.go | 16 ++++-- system/rest/request/auth.go | 46 +++++++++++++++++ system/rest/router.go | 2 +- system/types/credentials.go | 2 +- 17 files changed, 250 insertions(+), 60 deletions(-) diff --git a/api/system/spec.json b/api/system/spec.json index 85d029ba9..176f9b588 100644 --- a/api/system/spec.json +++ b/api/system/spec.json @@ -19,6 +19,22 @@ "path": "/check", "parameters": {} }, + { + "name": "exchangeAuthToken", + "method": "POST", + "title": "Exchange auth token for JWT", + "path": "/exchange", + "parameters": { + "post": [ + { + "name": "token", + "type": "string", + "required": true, + "title": "Token to be exchanged for JWT" + } + ] + } + }, { "name": "logout", "method": "GET", diff --git a/api/system/spec/auth.json b/api/system/spec/auth.json index f8ae9805a..6da64262a 100644 --- a/api/system/spec/auth.json +++ b/api/system/spec/auth.json @@ -21,6 +21,22 @@ "Path": "/check", "Parameters": {} }, + { + "Name": "exchangeAuthToken", + "Method": "POST", + "Title": "Exchange auth token for JWT", + "Path": "/exchange", + "Parameters": { + "post": [ + { + "name": "token", + "required": true, + "title": "Token to be exchanged for JWT", + "type": "string" + } + ] + } + }, { "Name": "logout", "Method": "GET", diff --git a/docs/Authentication.md b/docs/Authentication.md index 0e7e8b753..e51efb2e6 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -31,10 +31,10 @@ auth.external.providers.github.secret null auth.external.providers.linkedin.enabled false auth.external.providers.linkedin.key null auth.external.providers.linkedin.secret null -auth.external.providers.openid-connect.didmos2.enabled true -auth.external.providers.openid-connect.didmos2.key "tXM2ouiovowzGabk" -auth.external.providers.openid-connect.didmos2.issuer "https://satosa.didmos.latest.crust.tech" -auth.external.providers.openid-connect.didmos2.secret "e1d68bfd7718468ba8fd36131f5176b1" +auth.external.providers.openid-connect.crust-iam.enabled true +auth.external.providers.openid-connect.crust-iam.key "tXM2ouiovowzGabk" +auth.external.providers.openid-connect.crust-iam.issuer "https://satosa.didmos.latest.crust.tech" +auth.external.providers.openid-connect.crust-iam.secret "e1d68bfd7718468ba8fd36131f5176b1" auth.external.redirect-url "http://system.api.local.crust.tech:3002/auth/external/%s/callback" auth.external.session-store-secret "fCVFSRWjVEcoYuhXSf3f6zVWO1p38XEWz2yS8WH7wKDbvpxFrZq7zlEuiUTvk4QF" ``` @@ -52,7 +52,7 @@ On startup, you should see log entries similar to these: initializing external authentication providers (3) external authentication provider "facebook" added external authentication provider "gplus" added -external authentication provider "openid-connect.didmos2" added +external authentication provider "openid-connect.crust-iam" added ``` @@ -64,7 +64,7 @@ system-cli external-auth auto-discovery name url ``` ```bash -system-cli external-auth auto-discovery didmos2 https://satosa.didmos.crust.example.tld +system-cli external-auth auto-discovery crust-iam https://satosa.didmos.crust.example.tld ``` This will autodiscover and autoconfigure new OIDC provider. @@ -74,5 +74,5 @@ Please note that this provider is disabled by default. To enable it, run: ```bash -system-cli settings key auth.external.providers.openid-connect.didmos2.enabled true +system-cli settings key auth.external.providers.openid-connect.crust-iam.enabled true ``` diff --git a/docs/system/README.md b/docs/system/README.md index 5952e1abb..1976067bd 100644 --- a/docs/system/README.md +++ b/docs/system/README.md @@ -95,6 +95,7 @@ | ------ | -------- | ------- | | `GET` | `/auth/` | Returns auth settings | | `GET` | `/auth/check` | Check JWT token | +| `POST` | `/auth/exchange` | Exchange auth token for JWT | | `GET` | `/auth/logout` | Logout | ## Returns auth settings @@ -123,6 +124,20 @@ | Parameter | Type | Method | Description | Default | Required? | | --------- | ---- | ------ | ----------- | ------- | --------- | +## Exchange auth token for JWT + +#### Method + +| URI | Protocol | Method | Authentication | +| --- | -------- | ------ | -------------- | +| `/auth/exchange` | HTTP/S | POST | | + +#### Request parameters + +| Parameter | Type | Method | Description | Default | Required? | +| --------- | ---- | ------ | ----------- | ------- | --------- | +| token | string | POST | Token to be exchanged for JWT | N/A | YES | + ## Logout #### Method diff --git a/internal/auth/interfaces.go b/internal/auth/interfaces.go index f1179d34f..ad10152cc 100644 --- a/internal/auth/interfaces.go +++ b/internal/auth/interfaces.go @@ -1,9 +1,5 @@ package auth -import ( - "net/http" -) - type ( Identifiable interface { Identity() uint64 @@ -12,6 +8,5 @@ type ( TokenEncoder interface { Encode(identity Identifiable) string - SetCookie(w http.ResponseWriter, r *http.Request, identity Identifiable) } ) diff --git a/internal/auth/jwt.go b/internal/auth/jwt.go index c752b7d67..a90b052e6 100644 --- a/internal/auth/jwt.go +++ b/internal/auth/jwt.go @@ -68,22 +68,3 @@ func (t *token) Authenticator() func(http.Handler) http.Handler { }) } } - -// Extracts and authenticates JWT from context, validates claims -func (t *token) SetCookie(w http.ResponseWriter, r *http.Request, identity Identifiable) { - cookie := &http.Cookie{ - Name: "jwt", - Expires: time.Now().Add(time.Duration(t.expiry) * time.Minute), - Secure: r.URL.Scheme == "https", - Domain: t.cookieDomain, - Path: "/", - } - - if identity == nil { - cookie.Expires = time.Unix(0, 0) - } else { - cookie.Value = t.Encode(identity) - } - - http.SetCookie(w, cookie) -} diff --git a/internal/settings/settings.go b/internal/settings/settings.go index e0657b02a..10eb5bdff 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -64,7 +64,7 @@ func (s service) GetGlobalString(name string) (out string, err error) { const global = 0 var v *Value - if v, err = s.repository.Get(name, global); err == nil { + if v, err = s.repository.Get(name, global); err == nil && v != nil { err = v.Value.Unmarshal(&out) } @@ -75,7 +75,7 @@ func (s service) GetGlobalBool(name string) (out bool, err error) { const global = 0 var v *Value - if v, err = s.repository.Get(name, global); err == nil { + if v, err = s.repository.Get(name, global); err == nil && v != nil { err = v.Value.Unmarshal(&out) } diff --git a/system/cli/settings.go b/system/cli/settings.go index 0de84abaa..fb2fabadf 100644 --- a/system/cli/settings.go +++ b/system/cli/settings.go @@ -190,10 +190,14 @@ func settingsAutoConfigure(setSvc settings.Service, systemApiUrl, frontendUrl, f setIfMissing("auth.frontend.url.email-confirmation", func() interface{} { return frontendUrl + "/auth/confirm-email?token=" }) + + setIfMissing("auth.frontend.url.redirect", func() interface{} { + return frontendUrl + "/auth/" + }) } // Auth email (password reset, email confirmation) - setIfMissing("auth.frontend.url.email-confirmation", func() interface{} { + setIfMissing("auth.mail.from-address", func() interface{} { if len(fromAddress) > 0 { return fromAddress } diff --git a/system/internal/service/auth.go b/system/internal/service/auth.go index 379115026..851c1b9cd 100644 --- a/system/internal/service/auth.go +++ b/system/internal/service/auth.go @@ -35,12 +35,15 @@ type ( With(ctx context.Context) AuthService External(profile goth.User) (*types.User, error) + FrontendRedirectURL() string InternalSignUp(input *types.User, password string) (*types.User, error) InternalLogin(email string, password string) (*types.User, error) SetPassword(userID uint64, newPassword string) error ChangePassword(userID uint64, oldPassword, newPassword string) error + IssueAuthRequestToken(user *types.User) (token string, err error) + ValidateAuthRequestToken(token string) (user *types.User, err error) ValidateEmailConfirmationToken(token string) (user *types.User, err error) ExchangePasswordResetToken(token string) (user *types.User, exchangedToken string, err error) ValidatePasswordResetToken(token string) (user *types.User, err error) @@ -54,6 +57,7 @@ const ( credentialsTypeEmailAuthToken = "email-authentication-token" credentialsTypeResetPasswordToken = "password-reset-token" credentialsTypeResetPasswordTokenExchanged = "password-reset-token-exchanged" + credentialsTypeAuthToken = "auth-token" credentialsTokenLength = 32 ) @@ -227,6 +231,11 @@ func (svc *auth) External(profile goth.User) (u *types.User, err error) { }) } +// FrontendRedirectURL - a proxy to frontend redirect url setting +func (svc auth) FrontendRedirectURL() string { + return svc.settings.frontendUrlRedirect +} + // InternalSignUp protocol // // Forgiving but strict: valid existing users get notified @@ -549,6 +558,14 @@ func (svc auth) changePassword(userID uint64, hash []byte) (err error) { return errors.Wrap(err, "could not create new password") } +func (svc auth) IssueAuthRequestToken(user *types.User) (token string, err error) { + return svc.createUserToken(user, credentialsTypeAuthToken) +} + +func (svc auth) ValidateAuthRequestToken(token string) (user *types.User, err error) { + return svc.loadUserFromToken(token, credentialsTypeAuthToken) +} + func (svc auth) ValidateEmailConfirmationToken(token string) (user *types.User, err error) { if !svc.settings.internalEnabled { return nil, errors.New("internal authentication disabled") @@ -764,10 +781,22 @@ func (svc auth) validateToken(token string) (ID uint64, credentials string, err } func (svc auth) createUserToken(user *types.User, kind string) (token string, err error) { + var expiresAt time.Time + + switch kind { + case credentialsTypeAuthToken: + // 15 sec expiration for all tokens that are part of redirction + expiresAt = svc.now().Add(time.Second * 15) + default: + // 1h expiration for all tokens send via email + expiresAt = svc.now().Add(time.Minute * 60) + } + c, err := svc.credentials.Create(&types.Credentials{ OwnerID: user.ID, Kind: kind, Credentials: string(rand.Bytes(credentialsTokenLength)), + ExpiresAt: &expiresAt, }) if err != nil { diff --git a/system/internal/service/auth_settings.go b/system/internal/service/auth_settings.go index 031c4b864..d4c27b802 100644 --- a/system/internal/service/auth_settings.go +++ b/system/internal/service/auth_settings.go @@ -14,6 +14,9 @@ type ( // EmailAddress confirmation path ( "?token=" + ) frontendUrlEmailConfirmation string + // Where to redirect user after external auth flow + frontendUrlRedirect string + mailFromAddress string mailFromName string @@ -46,6 +49,7 @@ func AuthSettings(kv authSettingsStore) authSettings { return authSettings{ frontendUrlPasswordReset: kv.String("auth.frontend.url.password-reset"), frontendUrlEmailConfirmation: kv.String("auth.frontend.url.email-confirmation"), + frontendUrlRedirect: kv.String("auth.frontend.url.redirect"), mailFromAddress: kv.String("auth.mail.from-address"), mailFromName: kv.String("auth.mail.from-name"), @@ -78,6 +82,19 @@ func (s authSettings) Format() map[string]interface{} { label = strings.SplitN(p, ".", 2)[1] } + switch label { + case "crust-iam": + label = "Crust IAM" + case "facebook": + label = "Facebook" + case "gplus": + label = "Google" + case "linkedin": + label = "LinkedIn" + case "github": + label = "GitHub" + } + providers = append(providers, externalProvider{ Label: label, Handle: p, diff --git a/system/rest/auth.go b/system/rest/auth.go index b47d78c83..ea855f3c0 100644 --- a/system/rest/auth.go +++ b/system/rest/auth.go @@ -21,20 +21,28 @@ type ( Auth struct { jwt auth.TokenEncoder authSettings authServiceSettingsProvider + authSvc service.AuthService } authServiceSettingsProvider interface { Format() map[string]interface{} } + exchangeResponse struct { + JWT string `json:"jwt"` + User *outgoing.User `json:"user"` + } + checkResponse struct { User *outgoing.User `json:"user"` } ) -func (Auth) New() *Auth { +func (Auth) New(tenc auth.TokenEncoder) *Auth { return &Auth{ + jwt: tenc, authSettings: service.DefaultAuthSettings, + authSvc: service.DefaultAuth, } } @@ -43,17 +51,29 @@ func (ctrl *Auth) Check(ctx context.Context, r *request.AuthCheck) (interface{}, } func (ctrl *Auth) Logout(ctx context.Context, r *request.AuthLogout) (interface{}, error) { - return nil, errors.New("Not implemented: Auth.logout") + return nil, nil } func (ctrl *Auth) Settings(ctx context.Context, r *request.AuthSettings) (interface{}, error) { return ctrl.authSettings.Format(), nil } +func (ctrl *Auth) ExchangeAuthToken(ctx context.Context, r *request.AuthExchangeAuthToken) (interface{}, error) { + user, err := ctrl.authSvc.ValidateAuthRequestToken(r.Token) + if err != nil { + return nil, err + } + + return exchangeResponse{ + JWT: ctrl.jwt.Encode(user), + User: payload.User(user), + }, nil +} + // Handlers() func ignores "std" crust controllers // // Crush handlers are too abstract for our auth needs so we need (direct access to htt.ResponseWriter) -func (ctrl *Auth) Handlers(jwtEncoder auth.TokenEncoder) *handlers.Auth { +func (ctrl *Auth) Handlers() *handlers.Auth { h := handlers.NewAuth(ctrl) // Check JWT if valid h.Check = func(w http.ResponseWriter, r *http.Request) { @@ -61,8 +81,6 @@ func (ctrl *Auth) Handlers(jwtEncoder auth.TokenEncoder) *handlers.Auth { if identity := auth.GetIdentityFromContext(ctx); identity != nil && identity.Valid() { if user, err := service.DefaultUser.With(ctx).FindByID(identity.Identity()); err == nil { - jwtEncoder.SetCookie(w, r, user) - resputil.JSON(w, checkResponse{ User: payload.User(user), }) @@ -72,9 +90,5 @@ func (ctrl *Auth) Handlers(jwtEncoder auth.TokenEncoder) *handlers.Auth { } } - h.Logout = func(w http.ResponseWriter, r *http.Request) { - // nothing to do here... - } - return h } diff --git a/system/rest/auth_external.go b/system/rest/auth_external.go index 75b8b8f88..504be3d18 100644 --- a/system/rest/auth_external.go +++ b/system/rest/auth_external.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "net/http" + "net/url" "strings" "time" @@ -37,9 +38,9 @@ func NewSocial(jwtEncoder auth.TokenEncoder) *ExternalAuth { func (ctrl *ExternalAuth) MountRoutes(r chi.Router) { - // Make sure we're backwards compatible and redirect /oidc to /auth/external/openid-connect-didmos2 + // Make sure we're backwards compatible and redirect /oidc to /auth/external/openid-connect.crust-iam r.Get("/oidc", func(w http.ResponseWriter, req *http.Request) { - http.Redirect(w, req, externalAuthBaseUrl+"/openid-connect-didmos2", http.StatusMovedPermanently) + http.Redirect(w, req, externalAuthBaseUrl+"/openid-connect.crust-iam", http.StatusMovedPermanently) }) // Copy provider from path (Chi URL param) to request context and return it @@ -51,7 +52,8 @@ func (ctrl *ExternalAuth) MountRoutes(r chi.Router) { r.Get("/", func(w http.ResponseWriter, r *http.Request) { r = copyProviderToContext(r) - // Always set redir cookie, even if not requested. If param is empty, cookie is removed + // Always set redir cookie, even if not requested. + // If param is empty, cookie will be removed ctrl.setSessionCookie(w, r, "redir", r.URL.Query().Get("redir")) // try to get the user without re-authenticating @@ -102,22 +104,62 @@ func (ctrl *ExternalAuth) handleFailedCallback(w http.ResponseWriter, r *http.Re // Handles authentication via external auth providers of // unknown an user + appending authentication on external providers // to a current user +// +// +// Redirection rules: +// 1) use cookie (set from query-string param on first step +// 2) use `auth.frontend.url.redirect` setting +// 3) use current url func (ctrl *ExternalAuth) handleSuccessfulAuth(w http.ResponseWriter, r *http.Request, cred goth.User) { log.Printf("Successful external login: %v", cred) - if u, err := ctrl.auth.With(r.Context()).External(cred); err != nil { + svc := ctrl.auth.With(r.Context()) + + if u, err := svc.External(cred); err != nil { resputil.JSON(w, err) } else { - ctrl.jwtEncoder.SetCookie(w, r, u) - - if c, err := r.Cookie("redir"); c != nil && err == nil { - ctrl.setSessionCookie(w, r, "redir", "") - w.Header().Set("Location", c.Value) - w.WriteHeader(http.StatusSeeOther) + var ( + token string + redirUrl *url.URL + c *http.Cookie + ) + if c, err = r.Cookie("redir"); c != nil && err == nil { + if redirUrl, err = url.Parse(c.Value); err == nil { + // @todo validate origin/redir-domain + ctrl.setSessionCookie(w, r, "redir", "") + } + } else if fru := svc.FrontendRedirectURL(); fru != "" { + redirUrl, err = url.Parse(fru) + } else { + redirUrl = r.URL + } + + if err != nil { + resputil.JSON(w, err) + return + } + + if redirUrl != nil { + + q := redirUrl.Query() + + if u != nil { + if token, err = svc.IssueAuthRequestToken(u); err == nil { + q.Set("token", token) + } + } + + if err != nil { + q.Set("err", err.Error()) + } + + redirUrl.RawQuery = q.Encode() + + w.Header().Set("Location", redirUrl.String()) + w.WriteHeader(http.StatusSeeOther) } - resputil.JSON(w, u, err) } } diff --git a/system/rest/auth_internal.go b/system/rest/auth_internal.go index 4151db3b7..db95d72d6 100644 --- a/system/rest/auth_internal.go +++ b/system/rest/auth_internal.go @@ -124,5 +124,10 @@ func (ctrl *AuthInternal) ChangePassword(ctx context.Context, r *request.AuthInt return nil, errors.New("invalid user (not authenticated)") } - return nil, ctrl.authSvc.ChangePassword(identity.Identity(), r.OldPassword, r.NewPassword) + err := ctrl.authSvc.ChangePassword(identity.Identity(), r.OldPassword, r.NewPassword) + if err != nil { + return nil, err + } else { + return true, nil + } } diff --git a/system/rest/handlers/auth.go b/system/rest/handlers/auth.go index fb48a90fb..3663abff7 100644 --- a/system/rest/handlers/auth.go +++ b/system/rest/handlers/auth.go @@ -30,14 +30,16 @@ import ( type AuthAPI interface { Settings(context.Context, *request.AuthSettings) (interface{}, error) Check(context.Context, *request.AuthCheck) (interface{}, error) + ExchangeAuthToken(context.Context, *request.AuthExchangeAuthToken) (interface{}, error) Logout(context.Context, *request.AuthLogout) (interface{}, error) } // HTTP API interface type Auth struct { - Settings func(http.ResponseWriter, *http.Request) - Check func(http.ResponseWriter, *http.Request) - Logout func(http.ResponseWriter, *http.Request) + Settings func(http.ResponseWriter, *http.Request) + Check func(http.ResponseWriter, *http.Request) + ExchangeAuthToken func(http.ResponseWriter, *http.Request) + Logout func(http.ResponseWriter, *http.Request) } func NewAuth(ah AuthAPI) *Auth { @@ -56,6 +58,13 @@ func NewAuth(ah AuthAPI) *Auth { return ah.Check(r.Context(), params) }) }, + ExchangeAuthToken: func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + params := request.NewAuthExchangeAuthToken() + resputil.JSON(w, params.Fill(r), func() (interface{}, error) { + return ah.ExchangeAuthToken(r.Context(), params) + }) + }, Logout: func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() params := request.NewAuthLogout() @@ -71,6 +80,7 @@ func (ah *Auth) MountRoutes(r chi.Router, middlewares ...func(http.Handler) http r.Use(middlewares...) r.Get("/auth/", ah.Settings) r.Get("/auth/check", ah.Check) + r.Post("/auth/exchange", ah.ExchangeAuthToken) r.Get("/auth/logout", ah.Logout) }) } diff --git a/system/rest/request/auth.go b/system/rest/request/auth.go index ee3cff221..1028821a4 100644 --- a/system/rest/request/auth.go +++ b/system/rest/request/auth.go @@ -110,6 +110,52 @@ func (auReq *AuthCheck) Fill(r *http.Request) (err error) { var _ RequestFiller = NewAuthCheck() +// Auth exchangeAuthToken request parameters +type AuthExchangeAuthToken struct { + Token string +} + +func NewAuthExchangeAuthToken() *AuthExchangeAuthToken { + return &AuthExchangeAuthToken{} +} + +func (auReq *AuthExchangeAuthToken) Fill(r *http.Request) (err error) { + if strings.ToLower(r.Header.Get("content-type")) == "application/json" { + err = json.NewDecoder(r.Body).Decode(auReq) + + switch { + case err == io.EOF: + err = nil + case err != nil: + return errors.Wrap(err, "error parsing http request body") + } + } + + if err = r.ParseForm(); err != nil { + return err + } + + get := map[string]string{} + post := map[string]string{} + urlQuery := r.URL.Query() + for name, param := range urlQuery { + get[name] = string(param[0]) + } + postVars := r.Form + for name, param := range postVars { + post[name] = string(param[0]) + } + + if val, ok := post["token"]; ok { + + auReq.Token = val + } + + return err +} + +var _ RequestFiller = NewAuthExchangeAuthToken() + // Auth logout request parameters type AuthLogout struct { } diff --git a/system/rest/router.go b/system/rest/router.go index 1d40d79d7..1c44ac6fe 100644 --- a/system/rest/router.go +++ b/system/rest/router.go @@ -13,7 +13,7 @@ func MountRoutes(jwtEncoder auth.TokenEncoder) func(chi.Router) { NewSocial(jwtEncoder).MountRoutes(r) // Provide raw `/auth` handlers - Auth{}.New().Handlers(jwtEncoder).MountRoutes(r) + Auth{}.New(jwtEncoder).Handlers().MountRoutes(r) handlers.NewAuthInternal((AuthInternal{}).New(jwtEncoder)).MountRoutes(r) diff --git a/system/types/credentials.go b/system/types/credentials.go index ac327c879..eaaa1c998 100644 --- a/system/types/credentials.go +++ b/system/types/credentials.go @@ -23,5 +23,5 @@ type ( ) func (u *Credentials) Valid() bool { - return u.ID > 0 && (u.ExpiresAt == nil || u.ExpiresAt.Before(time.Now())) && u.DeletedAt == nil + return u.ID > 0 && (u.ExpiresAt == nil || u.ExpiresAt.After(time.Now())) && u.DeletedAt == nil }