connector: Connectors without a RefreshConnector should not return a refresh token instead of erroring

Author: rithujohn191

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
+}

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,

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