Skip to content

Commit

Permalink
refactor(client): OAuth2 Client IDs have UUID V4 enforced
Browse files Browse the repository at this point in the history
BREAKING CHANGE: It is no longer possible to set arbitrary client IDs. Instead, IDs have to be UUID V4 from now on. Clients created before the 2.0 release will continue to work with their legacy IDs.

Closes #2911
  • Loading branch information
aeneasr committed Jun 14, 2022
1 parent f6881c3 commit 540404e
Show file tree
Hide file tree
Showing 51 changed files with 659 additions and 450 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"client_id": "create-client-0",
"client_name": "",
"redirect_uris": null,
"grant_types": null,
Expand Down
12 changes: 7 additions & 5 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package client

import (
"github.com/ory/x/stringsx"
"strings"
"time"

Expand All @@ -46,11 +47,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.
Expand Down Expand Up @@ -252,7 +254,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 {
Expand Down
2 changes: 1 addition & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
22 changes: 15 additions & 7 deletions client/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ import (
"context"
"crypto/subtle"
"encoding/json"
"github.com/gofrs/uuid"
"github.com/ory/x/uuidx"
"io"
"net/http"
"time"

"github.com/pborman/uuid"

"github.com/ory/x/urlx"

"github.com/ory/fosite"
Expand Down Expand Up @@ -156,12 +156,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."))
}

if len(c.Secret) == 0 {
secretb, err := x.GenerateSecret(26)
if err != nil {
Expand All @@ -185,7 +193,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
Expand Down Expand Up @@ -226,7 +234,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
Expand Down Expand Up @@ -319,7 +327,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
Expand Down
Loading

0 comments on commit 540404e

Please sign in to comment.