refactor(client): OAuth2 Client IDs have UUID V4 enforced

BREAKING CHANGE: It is no longer possible to set arbitrary client IDs. Instead, OAuth2 Client IDs must be valid UUID V4. Clients created before the 2.0 release will continue to work with their legacy IDs.

Closes #2911

client/.snapshots/TestHandler-common-case=create_clients-case=1-description=basic_admin_registration.json

@@ -1,5 +1,5 @@
 {
-  "client_id": "create-client-2",
+  "client_id": "0f1bb84e-4405-4e93-950b-cdae88c5dbf6",
   "client_name": "",
   "client_secret": "averylongsecret",
   "redirect_uris": [
@@ -24,5 +24,5 @@
   "metadata": {
     "foo": "bar"
   },
-  "registration_client_uri": "http://localhost:4444/oauth2/register/create-client-2"
+  "registration_client_uri": "http://localhost:4444/oauth2/register/0f1bb84e-4405-4e93-950b-cdae88c5dbf6"
 }

client/.snapshots/TestHandler-common-case=create_clients-case=4-description=basic_dynamic_client_registration.json

@@ -1,4 +1,4 @@
 {
-  "error": "The requested action was forbidden",
+  "error": "The request was malformed or contained invalid parameters",
   "error_description": "It is not allowed to choose your own OAuth2 Client secret."
 }

client/.snapshots/TestHandler-common-case=create_clients-case=4-description=non-uuid_fails.json

@@ -0,0 +1,4 @@
+{
+  "error": "The request was malformed or contained invalid parameters",
+  "error_description": "Only UUID V4 (e.g. 8dcd6868-e294-4180-aa36-fbad26de79a6) can be chosen as OAuth2 Client IDs but got: not-a-uuid"
+}

client/.snapshots/TestHandler-common-case=create_clients-case=5-description=basic_dynamic_client_registration.json

@@ -0,0 +1,4 @@
+{
+  "error": "The request was malformed or contained invalid parameters",
+  "error_description": "It is not allowed to choose your own OAuth2 Client secret."
+}

client/.snapshots/TestHandler-common-case=create_clients-case=6-description=empty_ID_succeeds.json

@@ -0,0 +1,24 @@
+{
+  "client_name": "",
+  "client_secret": "averylongsecret",
+  "redirect_uris": [
+    "http://localhost:3000/cb"
+  ],
+  "grant_types": null,
+  "response_types": null,
+  "scope": "offline_access offline openid",
+  "audience": [],
+  "owner": "",
+  "policy_uri": "",
+  "allowed_cors_origins": [],
+  "tos_uri": "",
+  "client_uri": "",
+  "logo_uri": "",
+  "contacts": null,
+  "client_secret_expires_at": 0,
+  "subject_type": "public",
+  "jwks": {},
+  "token_endpoint_auth_method": "client_secret_basic",
+  "userinfo_signed_response_alg": "none",
+  "metadata": {}
+}

client/.snapshots/TestHandler-common-case=creating_a_client_dynamically_does_not_allow_setting_the_secret.json

@@ -1,7 +1,7 @@
 {
   "body": {
-    "error": "The requested action was forbidden",
+    "error": "The request was malformed or contained invalid parameters",
     "error_description": "It is not allowed to choose your own OAuth2 Client secret."
   },
-  "status": 403
+  "status": 400
 }

client/.snapshots/TestHandler-common-case=fetching_existing_client-endpoint=admin.json

@@ -1,6 +1,6 @@
 {
   "body": {
-    "client_id": "existing-client-fetch",
+    "client_id": "0e837115-5105-4da7-a85e-ac286c2ef50e",
     "client_name": "",
     "redirect_uris": [
       "http://localhost:3000/cb"

client/.snapshots/TestHandler-common-case=fetching_existing_client-endpoint=selfservice.json

@@ -1,6 +1,6 @@
 {
   "body": {
-    "client_id": "existing-client-fetch",
+    "client_id": "0e837115-5105-4da7-a85e-ac286c2ef50e",
     "client_name": "",
     "redirect_uris": [
       "http://localhost:3000/cb"

client/.snapshots/TestHandler-common-case=updating_existing_client-endpoint=admin.json

@@ -1,6 +1,6 @@
 {
   "body": {
-    "client_id": "update-existing-client-admin",
+    "client_id": "c614c65a-72f3-4dd4-8217-f4c6343533dc",
     "client_name": "",
     "client_secret": "averylongsecret",
     "redirect_uris": [

client/.snapshots/TestHandler-common-case=updating_existing_client-endpoint=dynamic_client_registration.json

@@ -1,6 +1,6 @@
 {
   "body": {
-    "client_id": "update-existing-client-selfservice",
+    "client_id": "b33d7cff-ecc9-4acf-9ce7-67436bc763d4",
     "client_name": "",
     "redirect_uris": [
       "http://localhost:3000/cb",

client/.snapshots/TestHandler-create_client_registration_tokens-case=0-dynamic=false.json

@@ -1,5 +1,4 @@
 {
-  "client_id": "create-client-0",
   "client_name": "",
   "redirect_uris": null,
   "grant_types": null,
@@ -16,6 +15,5 @@
   "client_secret_expires_at": 0,
   "subject_type": "",
   "jwks": {},
-  "metadata": {},
-  "registration_client_uri": "http://localhost:4444/oauth2/register/create-client-0"
+  "metadata": {}
 }

client/.snapshots/TestHandler-create_client_registration_tokens-case=2-dynamic=false.json

@@ -1,5 +1,4 @@
 {
-  "client_id": "create-client-1",
   "client_name": "",
   "redirect_uris": null,
   "grant_types": null,
@@ -16,6 +15,5 @@
   "client_secret_expires_at": 0,
   "subject_type": "",
   "jwks": {},
-  "metadata": {},
-  "registration_client_uri": "http://localhost:4444/oauth2/register/create-client-1"
+  "metadata": {}
 }

client/.snapshots/TestHandler-create_client_registration_tokens-case=3-dynamic=false.json

@@ -1,7 +1,6 @@
 {
-  "client_id": "",
   "client_name": "",
-  "client_secret": "create-client-2",
+  "client_secret": "01bbf13a-ae3e-44d5-b4b4-dd78137041be",
   "redirect_uris": null,
   "grant_types": null,
   "response_types": null,
@@ -17,6 +16,5 @@
   "client_secret_expires_at": 0,
   "subject_type": "",
   "jwks": {},
-  "metadata": {},
-  "registration_client_uri": "http://localhost:4444/oauth2/register/"
+  "metadata": {}
 }

client/client.go

@@ -24,6 +24,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/ory/x/stringsx"
+
 	"github.com/gobuffalo/pop/v6"
 	"github.com/gofrs/uuid"
 
@@ -46,11 +48,12 @@ type Client struct {
 	ID  uuid.UUID `json:"-" db:"pk"`
 	NID uuid.UUID `db:"nid" faker:"-" json:"-"`
 
-	// This field is deprecated and will be removed
-	PKDeprecated int64 `json:"-" db:"pk_deprecated"`
+	// ID is the id for this client.
+	LegacyClientID string `json:"client_id" db:"id"`
 
-	// ID  is the id for this client.
-	OutfacingID string `json:"client_id" db:"id"`
+	// DEPRECATED: This field is deprecated and will be removed. It serves
+	// no purpose except the database not complaining.
+	PKDeprecated int64 `json:"-" db:"pk_deprecated"`
 
 	// Name is the human-readable string name of the client to be presented to the
 	// end-user during authorization.
@@ -252,7 +255,7 @@ func (c *Client) BeforeSave(_ *pop.Connection) error {
 }
 
 func (c *Client) GetID() string {
-	return c.OutfacingID
+	return stringsx.Coalesce(c.LegacyClientID, c.ID.String())
 }
 
 func (c *Client) GetRedirectURIs() []string {

client/client_test.go

@@ -33,7 +33,7 @@ var _ fosite.Client = new(Client)
 
 func TestClient(t *testing.T) {
 	c := &Client{
-		OutfacingID:             "foo",
+		LegacyClientID:          "foo",
 		RedirectURIs:            []string{"foo"},
 		Scope:                   "foo bar",
 		TokenEndpointAuthMethod: "none",

client/handler.go

@@ -28,7 +28,9 @@ import (
 	"net/http"
 	"time"
 
-	"github.com/pborman/uuid"
+	"github.com/gofrs/uuid"
+
+	"github.com/ory/x/uuidx"
 
 	"github.com/ory/x/urlx"
 
@@ -156,12 +158,20 @@ func (h *Handler) CreateClient(r *http.Request, validator func(context.Context,
 	}
 
 	if isDynamic {
-		c.OutfacingID = uuid.New()
+		c.LegacyClientID = uuidx.NewV4().String()
 		if c.Secret != "" {
-			return nil, errorsx.WithStack(herodot.ErrForbidden.WithReasonf("It is not allowed to choose your own OAuth2 Client secret."))
+			return nil, errorsx.WithStack(herodot.ErrBadRequest.WithReasonf("It is not allowed to choose your own OAuth2 Client secret."))
 		}
 	}
 
+	if c.LegacyClientID == "" {
+		c.LegacyClientID = uuidx.NewV4().String()
+	}
+
+	if _, err := uuid.FromString(c.LegacyClientID); err != nil {
+		return nil, errorsx.WithStack(herodot.ErrBadRequest.WithReasonf("Only UUID V4 (e.g. 8dcd6868-e294-4180-aa36-fbad26de79a6) can be chosen as OAuth2 Client IDs but got: %s", c.LegacyClientID))
+	}
+
 	if len(c.Secret) == 0 {
 		secretb, err := x.GenerateSecret(26)
 		if err != nil {
@@ -185,7 +195,7 @@ func (h *Handler) CreateClient(r *http.Request, validator func(context.Context,
 
 	c.RegistrationAccessToken = token
 	c.RegistrationAccessTokenSignature = signature
-	c.RegistrationClientURI = urlx.AppendPaths(h.r.Config().PublicURL(r.Context()), DynClientsHandlerPath+"/"+c.OutfacingID).String()
+	c.RegistrationClientURI = urlx.AppendPaths(h.r.Config().PublicURL(r.Context()), DynClientsHandlerPath+"/"+c.GetID()).String()
 
 	if err := h.r.ClientManager().CreateClient(r.Context(), &c); err != nil {
 		return nil, err
@@ -226,7 +236,7 @@ func (h *Handler) Update(w http.ResponseWriter, r *http.Request, ps httprouter.P
 		return
 	}
 
-	c.OutfacingID = ps.ByName("id")
+	c.LegacyClientID = ps.ByName("id")
 	if err := h.updateClient(r.Context(), &c, h.r.ClientValidator().Validate); err != nil {
 		h.r.Writer().WriteError(w, r, err)
 		return
@@ -319,7 +329,7 @@ func (h *Handler) UpdateDynamicRegistration(w http.ResponseWriter, r *http.Reque
 	c.RegistrationAccessToken = token
 	c.RegistrationAccessTokenSignature = signature
 
-	c.OutfacingID = client.GetID()
+	c.LegacyClientID = client.GetID()
 	if err := h.updateClient(r.Context(), &c, h.r.ClientValidator().ValidateDynamicRegistration); err != nil {
 		h.r.Writer().WriteError(w, r, err)
 		return