Resolve comments.

api/client/client.go

@@ -1423,8 +1423,13 @@ func (c *Client) GetOIDCConnector(ctx context.Context, name string, withSecrets
 	if err != nil {
 		return nil, trail.FromGRPC(err)
 	}
+
+	// Set RedirectURLs if not already set
 	// DELETE IN 11.0.0
-	resp.CheckAndSetRedirectURLs()
+	if err := resp.CheckAndSetDefaults(); err != nil {
+		return nil, trace.Wrap(err)
+	}
+
 	return resp, nil
 }
 
@@ -1437,8 +1442,11 @@ func (c *Client) GetOIDCConnectors(ctx context.Context, withSecrets bool) ([]typ
 	}
 	oidcConnectors := make([]types.OIDCConnector, len(resp.OIDCConnectors))
 	for i, oidcConnector := range resp.OIDCConnectors {
+		// Set RedirectURLs if not already set
 		// DELETE IN 11.0.0
-		oidcConnector.CheckAndSetRedirectURLs()
+		if err := oidcConnector.CheckAndSetDefaults(); err != nil {
+			return nil, trace.Wrap(err)
+		}
 		oidcConnectors[i] = oidcConnector
 	}
 	return oidcConnectors, nil
@@ -1450,8 +1458,6 @@ func (c *Client) UpsertOIDCConnector(ctx context.Context, oidcConnector types.OI
 	if !ok {
 		return trace.BadParameter("invalid type %T", oidcConnector)
 	}
-	// DELETE IN 11.0.0
-	connector.CheckAndSetRedirectURL()
 	_, err := c.grpc.UpsertOIDCConnector(ctx, connector, c.callOpts...)
 	return trail.FromGRPC(err)
 }

api/types/oidc.go

@@ -367,6 +367,14 @@ func (o *OIDCConnectorV3) CheckAndSetDefaults() error {
 		return trace.BadParameter("IssuerURL: bad url: '%v'", o.GetIssuerURL())
 	}
 
+	// RedirectURL must be checked/set when communicating with an old server or client.
+	// DELETE IN 11.0.0
+	if o.Spec.RedirectURL == "" && len(o.Spec.RedirectURLs) != 0 {
+		o.Spec.RedirectURL = o.Spec.RedirectURLs[0]
+	} else if len(o.Spec.RedirectURLs) == 0 && o.Spec.RedirectURL != "" {
+		o.Spec.RedirectURLs = []string{o.Spec.RedirectURL}
+	}
+
 	if len(o.GetRedirectURLs()) == 0 {
 		return trace.BadParameter("RedirectURL: missing redirect_url")
 	}
@@ -401,27 +409,3 @@ func (o *OIDCConnectorV3) CheckAndSetDefaults() error {
 
 	return nil
 }
-
-// CheckAndSetRedirectURL checks if RedirectURL is set, and sets it to
-// RedirectURLs[0] if it isn't. This is used when the connector
-// is sent in a request/response to an old server/client respectively.
-//
-// DELETE IN 11.0.0
-func (o *OIDCConnectorV3) CheckAndSetRedirectURL() {
-	if o.Spec.RedirectURL == "" && len(o.Spec.RedirectURLs) != 0 {
-		o.Spec.RedirectURL = o.Spec.RedirectURLs[0]
-	}
-}
-
-// CheckAndSetRedirectURLs checks if RedirectURLs is set, and sets its first
-// element to RedirectURL if it's empty. This is used when the connector
-// is received from a request/response of an old client/server respectively.
-//
-// DELETE IN 11.0.0
-func (o *OIDCConnectorV3) CheckAndSetRedirectURLs() {
-	if len(o.Spec.RedirectURLs) == 0 && o.Spec.RedirectURL != "" {
-		o.Spec.RedirectURLs = []string{o.Spec.RedirectURL}
-	}
-	// unset deprectated field to prevent it from being unmarshalled in yaml
-	o.Spec.RedirectURL = ""
-}

lib/auth/grpcserver.go

@@ -2226,8 +2226,6 @@ func (g *GRPCServer) GetOIDCConnector(ctx context.Context, req *types.ResourceWi
 	if !ok {
 		return nil, trace.Errorf("encountered unexpected OIDC connector type %T", oc)
 	}
-	// DELETE IN 11.0.0
-	connector.CheckAndSetRedirectURL()
 	return connector, nil
 }
 
@@ -2247,8 +2245,6 @@ func (g *GRPCServer) GetOIDCConnectors(ctx context.Context, req *types.Resources
 		if connectors[i], ok = oc.(*types.OIDCConnectorV3); !ok {
 			return nil, trace.Errorf("encountered unexpected OIDC connector type %T", oc)
 		}
-		// DELETE IN 11.0.0
-		connectors[i].CheckAndSetRedirectURL()
 	}
 	return &types.OIDCConnectorV3List{
 		OIDCConnectors: connectors,
@@ -2261,11 +2257,6 @@ func (g *GRPCServer) UpsertOIDCConnector(ctx context.Context, oidcConnector *typ
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	// DELETE IN 11.0.0
-	oidcConnector.CheckAndSetRedirectURLs()
-	if err = oidcConnector.CheckAndSetDefaults(); err != nil {
-		return nil, trace.Wrap(err)
-	}
 	if err = auth.ServerWithRoles.UpsertOIDCConnector(ctx, oidcConnector); err != nil {
 		return nil, trace.Wrap(err)
 	}

lib/auth/oidc.go

@@ -49,7 +49,7 @@ func (a *Server) getOIDCConnectorAndClient(ctx context.Context, request services
 	// stateless test flow
 	if request.SSOTestFlow {
 		if request.ConnectorSpec == nil {
-			return nil, nil, trace.BadParameter("ConnectorSpec cannot be nil when SSOTestFlow is tru")
+			return nil, nil, trace.BadParameter("ConnectorSpec cannot be nil when SSOTestFlow is true")
 		}
 
 		if request.ConnectorID == "" {

lib/services/oidc.go

@@ -61,9 +61,9 @@ func GetRedirectURL(conn types.OIDCConnector, proxyAddr string) (string, error)
 		return "", trace.BadParameter("No redirect URLs provided")
 	}
 
-	// If a specific proxyAddr wasn't provided in the oidc
-	// auth request, just use the default redirect URL.
-	if proxyAddr == "" {
+	// If a specific proxyAddr wasn't provided in the oidc auth request,
+	// or there is only one redirect URL, use the first redirect URL.
+	if proxyAddr == "" || len(conn.GetRedirectURLs()) == 1 {
 		return conn.GetRedirectURLs()[0], nil
 	}
 
@@ -95,6 +95,7 @@ func GetRedirectURL(conn types.OIDCConnector, proxyAddr string) (string, error)
 		return matchingHostname, nil
 	}
 
+	// No match, default to the first redirect URL.
 	return conn.GetRedirectURLs()[0], nil
 }