X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/fb348442584fc62bb670c378f80ab4c943e875cc..HEAD:/lib/controller/localdb/login_oidc.go diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go index 6d6f80f39c..d91cdddc01 100644 --- a/lib/controller/localdb/login_oidc.go +++ b/lib/controller/localdb/login_oidc.go @@ -14,8 +14,10 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/url" + "regexp" "strings" "sync" "text/template" @@ -28,7 +30,7 @@ import ( "git.arvados.org/arvados.git/sdk/go/auth" "git.arvados.org/arvados.git/sdk/go/ctxlog" "git.arvados.org/arvados.git/sdk/go/httpserver" - "github.com/coreos/go-oidc" + "github.com/coreos/go-oidc/v3/oidc" lru "github.com/hashicorp/golang-lru" "github.com/jmoiron/sqlx" "github.com/lib/pq" @@ -66,10 +68,11 @@ type oidcLoginController struct { // https://people.googleapis.com/) peopleAPIBasePath string - provider *oidc.Provider // initialized by setup() - oauth2conf *oauth2.Config // initialized by setup() - verifier *oidc.IDTokenVerifier // initialized by setup() - mu sync.Mutex // protects setup() + provider *oidc.Provider // initialized by setup() + endSessionURL *url.URL // initialized by setup() + oauth2conf *oauth2.Config // initialized by setup() + verifier *oidc.IDTokenVerifier // initialized by setup() + mu sync.Mutex // protects setup() } // Initialize ctrl.provider and ctrl.oauth2conf. @@ -99,11 +102,46 @@ func (ctrl *oidcLoginController) setup() error { ClientID: ctrl.ClientID, }) ctrl.provider = provider + var claims struct { + EndSessionEndpoint string `json:"end_session_endpoint"` + } + err = provider.Claims(&claims) + if err != nil { + return fmt.Errorf("error parsing OIDC discovery metadata: %v", err) + } else if claims.EndSessionEndpoint == "" { + ctrl.endSessionURL = nil + } else { + u, err := url.Parse(claims.EndSessionEndpoint) + if err != nil { + return fmt.Errorf("OIDC end_session_endpoint is not a valid URL: %v", err) + } else if u.Scheme != "https" { + return fmt.Errorf("OIDC end_session_endpoint MUST use HTTPS but does not: %v", u.String()) + } else { + ctrl.endSessionURL = u + } + } return nil } func (ctrl *oidcLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) { - return logout(ctx, ctrl.Cluster, opts) + err := ctrl.setup() + if err != nil { + return arvados.LogoutResponse{}, fmt.Errorf("error setting up OpenID Connect provider: %s", err) + } + resp, err := logout(ctx, ctrl.Cluster, opts) + if err != nil { + return arvados.LogoutResponse{}, err + } + creds, credsOK := auth.FromContext(ctx) + if ctrl.endSessionURL != nil && credsOK && len(creds.Tokens) > 0 { + values := ctrl.endSessionURL.Query() + values.Set("client_id", ctrl.ClientID) + values.Set("post_logout_redirect_uri", resp.RedirectLocation) + u := *ctrl.endSessionURL + u.RawQuery = values.Encode() + resp.RedirectLocation = u.String() + } + return resp, err } func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) { @@ -116,6 +154,9 @@ func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOp if opts.ReturnTo == "" { return loginError(errors.New("missing return_to parameter")) } + if err := validateLoginRedirectTarget(ctrl.Parent.cluster, opts.ReturnTo); err != nil { + return loginError(fmt.Errorf("invalid return_to parameter: %s", err)) + } state := ctrl.newOAuth2State([]byte(ctrl.Cluster.SystemRootToken), opts.Remote, opts.ReturnTo) var authparams []oauth2.AuthCodeOption for k, v := range ctrl.AuthParams { @@ -149,10 +190,39 @@ func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOp return loginError(err) } ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{ctrl.Cluster.SystemRootToken}}) - return ctrl.Parent.UserSessionCreate(ctxRoot, rpc.UserSessionCreateOptions{ - ReturnTo: state.Remote + "," + state.ReturnTo, + resp, err := ctrl.Parent.UserSessionCreate(ctxRoot, rpc.UserSessionCreateOptions{ + ReturnTo: state.Remote + ",https://controller.api.client.invalid", AuthInfo: *authinfo, }) + if err != nil { + return resp, err + } + // Extract token from rails' UserSessionCreate response, and + // attach it to our caller's desired ReturnTo URL. The Rails + // handler explicitly disallows sending the real ReturnTo as a + // belt-and-suspenders defence against Rails accidentally + // exposing an additional login relay. + u, err := url.Parse(resp.RedirectLocation) + if err != nil { + return resp, err + } + token := u.Query().Get("api_token") + if token == "" { + resp.RedirectLocation = state.ReturnTo + } else { + u, err := url.Parse(state.ReturnTo) + if err != nil { + return resp, err + } + q := u.Query() + if q == nil { + q = url.Values{} + } + q.Set("api_token", token) + u.RawQuery = q.Encode() + resp.RedirectLocation = u.String() + } + return resp, nil } func (ctrl *oidcLoginController) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) { @@ -335,7 +405,7 @@ func OIDCAccessTokenAuthorizer(cluster *arvados.Cluster, getdb func(context.Cont // We want ctrl to be nil if the chosen controller is not a // *oidcLoginController, so we can ignore the 2nd return value // of this type cast. - ctrl, _ := NewConn(cluster).loginController.(*oidcLoginController) + ctrl, _ := NewConn(context.Background(), cluster, getdb).loginController.(*oidcLoginController) cache, err := lru.New2Q(tokenCacheSize) if err != nil { panic(err) @@ -390,6 +460,9 @@ func (ta *oidcTokenAuthorizer) WrapCalls(origFunc api.RoutableFunc) api.Routable } } +// Matches error from oidc UserInfo() when receiving HTTP status 5xx +var re5xxError = regexp.MustCompile(`^5\d\d `) + // registerToken checks whether tok is a valid OIDC Access Token and, // if so, ensures that an api_client_authorizations row exists so that // RailsAPI will accept it as an Arvados token. @@ -459,6 +532,7 @@ func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) er return fmt.Errorf("error setting up OpenID Connect provider: %s", err) } if ok, err := ta.checkAccessTokenScope(ctx, tok); err != nil || !ok { + // Note checkAccessTokenScope logs any interesting errors ta.cache.Add(tok, time.Now().Add(tokenCacheNegativeTTL)) return err } @@ -467,6 +541,21 @@ func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) er } userinfo, err := ta.ctrl.provider.UserInfo(ctx, oauth2.StaticTokenSource(oauth2Token)) if err != nil { + if neterr := net.Error(nil); errors.As(err, &neterr) || re5xxError.MatchString(err.Error()) { + // If this token is in fact a valid OIDC + // token, but we failed to validate it here + // because of a network problem or internal + // server error, we error out now with a 5xx + // error, indicating to the client that they + // can try again. If we didn't error out now, + // the unrecognized token would eventually + // cause a 401 error further down the stack, + // which the caller would interpret as an + // unrecoverable failure. + ctxlog.FromContext(ctx).WithError(err).Debugf("treating OIDC UserInfo lookup error type %T as transient; failing request instead of forwarding token blindly", err) + return err + } + ctxlog.FromContext(ctx).WithError(err).WithField("HMAC", hmac).Debug("UserInfo failed (not an OIDC token?), caching negative result") ta.cache.Add(tok, time.Now().Add(tokenCacheNegativeTTL)) return nil } @@ -560,6 +649,6 @@ func (ta *oidcTokenAuthorizer) checkAccessTokenScope(ctx context.Context, tok st return true, nil } } - ctxlog.FromContext(ctx).WithFields(logrus.Fields{"have": claims.Scope, "need": ta.ctrl.AcceptAccessTokenScope}).Infof("unacceptable access token scope") + ctxlog.FromContext(ctx).WithFields(logrus.Fields{"have": claims.Scope, "need": ta.ctrl.AcceptAccessTokenScope}).Info("unacceptable access token scope") return false, httpserver.ErrorWithStatus(errors.New("unacceptable access token scope"), http.StatusUnauthorized) }