From 59502850f0b6a070322d415627f603704a4a5899 Mon Sep 17 00:00:00 2001 From: rithu john Date: Thu, 23 Mar 2017 14:06:30 -0700 Subject: [PATCH] connector: Connectors without a RefreshConnector should not return a refresh token instead of erroring --- connector/oidc/oidc.go | 6 ++++++ connector/saml/saml.go | 6 ------ server/handlers.go | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 0310266c..db9c641f 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -117,6 +117,7 @@ func (c *Config) Open(logger logrus.FieldLogger) (conn connector.Connector, err var ( _ connector.CallbackConnector = (*oidcConnector)(nil) + _ connector.RefreshConnector = (*oidcConnector)(nil) ) type oidcConnector struct { @@ -188,3 +189,8 @@ func (c *oidcConnector) HandleCallback(s connector.Scopes, r *http.Request) (ide } return identity, nil } + +// Refresh is implemented for backwards compatibility, even though it's a no-op. +func (c *oidcConnector) Refresh(ctx context.Context, s connector.Scopes, identity connector.Identity) (connector.Identity, error) { + return identity, nil +} diff --git a/connector/saml/saml.go b/connector/saml/saml.go index a7b01734..7fad1cc3 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -241,12 +241,6 @@ type provider struct { func (p *provider) POSTData(s connector.Scopes, id string) (action, value string, err error) { - // NOTE(ericchiang): If we can't follow up with the identity provider, can we - // support refresh tokens? - if s.OfflineAccess { - return "", "", fmt.Errorf("SAML does not support offline access") - } - r := &authnRequest{ ProtocolBinding: bindingPOST, ID: id, diff --git a/server/handlers.go b/server/handlers.go index c7da2d5b..5bd469c6 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -646,6 +646,20 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s } reqRefresh := func() bool { + // Ensure the connector supports refresh tokens. + // + // Connectors like `samlExperimental` do not implement RefreshConnector. + conn, ok := s.connectors[authCode.ConnectorID] + if !ok { + s.logger.Errorf("connector ID not found: %q", authCode.ConnectorID) + s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) + return false + } + _, ok = conn.Connector.(connector.RefreshConnector) + if !ok { + return false + } + for _, scope := range authCode.Scopes { if scope == scopeOfflineAccess { return true