Skip to content

Commit

Permalink
fix: params for oauth client manager (#1414)
Browse files Browse the repository at this point in the history 
Signed-off-by: Andrii Holovko <andriy.holovko@gmail.com>
  • Loading branch information
@aholovko
aholovko authored Sep 12, 2023
1 parent 53bc2ef commit 693c498
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 129 deletions.
40 changes: 35 additions & 5 deletions pkg/oauth2client/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@ import (
var _ fosite.Client = (*Client)(nil)

const (
defaultGrantType = "authorization_code"
defaultResponseType = "code"
GrantTypeAuthorizationCode = "authorization_code"
GrantTypePreAuthorizedCode = "urn:ietf:params:oauth:grant-type:pre-authorized_code"

ResponseTypeCode = "code"

TokenEndpointAuthMethodNone = "none"
TokenEndpointAuthMethodClientSecretBasic = "client_secret_basic"
TokenEndpointAuthMethodClientSecretPost = "client_secret_post"
)

// Client represents an OAuth2 client.
Expand All @@ -26,7 +32,7 @@ type Client struct {
Name string `json:"client_name"`
URI string `json:"client_uri"`
Secret []byte `json:"client_secret,omitempty"`
SecretExpiresAt time.Time `json:"client_secret_expires_at,omitempty"`
SecretExpiresAt int64 `json:"client_secret_expires_at,omitempty"`
RotatedSecrets [][]byte `json:"rotated_secrets,omitempty"`
RedirectURIs []string `json:"redirect_uris"`
GrantTypes []string `json:"grant_types"`
Expand Down Expand Up @@ -63,7 +69,7 @@ func (c *Client) GetRedirectURIs() []string {
// GetGrantTypes returns the client grant types.
func (c *Client) GetGrantTypes() fosite.Arguments {
if len(c.GrantTypes) == 0 {
return fosite.Arguments{defaultGrantType}
return fosite.Arguments{GrantTypeAuthorizationCode}
}

return c.GrantTypes
Expand All @@ -72,7 +78,7 @@ func (c *Client) GetGrantTypes() fosite.Arguments {
// GetResponseTypes returns the client response types.
func (c *Client) GetResponseTypes() fosite.Arguments {
if len(c.ResponseTypes) == 0 {
return fosite.Arguments{defaultResponseType}
return fosite.Arguments{ResponseTypeCode}
}

return c.ResponseTypes
Expand All @@ -92,3 +98,27 @@ func (c *Client) IsPublic() bool {
func (c *Client) GetAudience() fosite.Arguments {
return c.Audience
}

// GrantTypesSupported returns grant types supported by the VCS OIDC provider.
func GrantTypesSupported() []string {
return []string{
GrantTypeAuthorizationCode,
GrantTypePreAuthorizedCode,
}
}

// ResponseTypesSupported returns response types supported by the VCS OIDC provider.
func ResponseTypesSupported() []string {
return []string{
ResponseTypeCode,
}
}

// TokenEndpointAuthMethodsSupported returns client authentication methods supported by the VCS token endpoint.
func TokenEndpointAuthMethodsSupported() []string {
return []string{
TokenEndpointAuthMethodNone,
TokenEndpointAuthMethodClientSecretBasic,
TokenEndpointAuthMethodClientSecretPost,
}
}
27 changes: 13 additions & 14 deletions pkg/profile/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,19 @@ type CredentialMetaData struct {

// OIDCConfig represents issuer's OIDC configuration.
type OIDCConfig struct {
IssuerWellKnownURL string `json:"issuer_well_known"`
ClientID string `json:"client_id"`
ClientSecretHandle string `json:"client_secret_handle"`
ScopesSupported []string `json:"scopes_supported"`
GrantTypesSupported []string `json:"grant_types_supported"`
ResponseTypesSupported []string `json:"response_types_supported"`
TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported"`
EnableDynamicClientRegistration bool `json:"enable_dynamic_client_registration"`
EnableDiscoverableClientIDScheme bool `json:"enable_discoverable_client_id_scheme"`
InitialAccessTokenLifespan time.Duration `json:"initial_access_token_lifespan"`
PreAuthorizedGrantAnonymousAccessSupported bool `json:"pre-authorized_grant_anonymous_access_supported"`
WalletInitiatedAuthFlowSupported bool `json:"wallet_initiated_auth_flow_supported"`
SignedCredentialOfferSupported bool `json:"signed_credential_offer_supported"`
ClaimsEndpoint string `json:"claims_endpoint"`
IssuerWellKnownURL string `json:"issuer_well_known"`
ClientID string `json:"client_id"`
ClientSecretHandle string `json:"client_secret_handle"`
ScopesSupported []string `json:"scopes_supported"`
GrantTypesSupported []string `json:"grant_types_supported"`
ResponseTypesSupported []string `json:"response_types_supported"`
TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported"`
EnableDynamicClientRegistration bool `json:"enable_dynamic_client_registration"`
EnableDiscoverableClientIDScheme bool `json:"enable_discoverable_client_id_scheme"`
PreAuthorizedGrantAnonymousAccessSupported bool `json:"pre-authorized_grant_anonymous_access_supported"`
WalletInitiatedAuthFlowSupported bool `json:"wallet_initiated_auth_flow_supported"`
SignedCredentialOfferSupported bool `json:"signed_credential_offer_supported"`
ClaimsEndpoint string `json:"claims_endpoint"`
}

// VCConfig describes how to sign verifiable credentials.
Expand Down
2 changes: 1 addition & 1 deletion pkg/restapi/v1/oidc4ci/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ func (c *Controller) OidcRegisterClient(e echo.Context, profileID string, profil

if client.Secret != nil {
resp.ClientSecret = lo.ToPtr(string(client.Secret))
resp.ClientSecretExpiresAt = lo.ToPtr(int(client.SecretExpiresAt.Unix()))
resp.ClientSecretExpiresAt = lo.ToPtr(int(client.SecretExpiresAt))
}

if client.Name != "" {
Expand Down
2 changes: 1 addition & 1 deletion pkg/restapi/v1/oidc4ci/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2105,7 +2105,7 @@ func TestController_OidcRegisterClient(t *testing.T) {
Name: "client-name",
URI: "https://example.com",
Secret: []byte("secret"),
SecretExpiresAt: time.Now().Add(5 * time.Minute),
SecretExpiresAt: 0,
RedirectURIs: []string{"https://example.com/callback"},
GrantTypes: []string{"authorization_code"},
ResponseTypes: []string{"code"},
Expand Down
39 changes: 6 additions & 33 deletions pkg/service/clientmanager/client_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ import (

var _ ServiceInterface = (*Manager)(nil)

const (
defaultInitialAccessTokenLifespan = 5 * time.Minute
defaultTokenEndpointAuthMethod = "client_secret_basic"
)

type store interface {
InsertClient(ctx context.Context, client *oauth2client.Client) (string, error)
GetClient(ctx context.Context, id string) (fosite.Client, error)
Expand Down Expand Up @@ -118,36 +113,31 @@ func (m *Manager) Create(ctx context.Context, profileID, profileVersion string,
return nil, InvalidClientMetadataError("scope", err)
}

if err = setGrantTypes(client, profile.OIDCConfig.GrantTypesSupported, data.GrantTypes); err != nil {
if err = setGrantTypes(client, oauth2client.GrantTypesSupported(), data.GrantTypes); err != nil {
return nil, InvalidClientMetadataError("grant_types", err)
}

if err = setResponseTypes(client, profile.OIDCConfig.ResponseTypesSupported, data.ResponseTypes); err != nil {
if err = setResponseTypes(client, oauth2client.ResponseTypesSupported(), data.ResponseTypes); err != nil {
return nil, InvalidClientMetadataError("response_types", err)
}

if err = setTokenEndpointAuthMethod(
client,
profile.OIDCConfig.TokenEndpointAuthMethodsSupported,
oauth2client.TokenEndpointAuthMethodsSupported(),
data.TokenEndpointAuthMethod,
); err != nil {
return nil, InvalidClientMetadataError("token_endpoint_auth_method", err)
}

if client.TokenEndpointAuthMethod != "none" {
if client.TokenEndpointAuthMethod != oauth2client.TokenEndpointAuthMethodNone {
var secret []byte

if secret, err = generateSecret(); err != nil {
return nil, err
}

client.Secret = secret

if profile.OIDCConfig.InitialAccessTokenLifespan != 0 {
client.SecretExpiresAt = time.Now().Add(profile.OIDCConfig.InitialAccessTokenLifespan)
} else {
client.SecretExpiresAt = time.Now().Add(defaultInitialAccessTokenLifespan)
}
client.SecretExpiresAt = 0 // never expires
}

if err = setJSONWebKeys(client, data.JSONWebKeys); err != nil {
Expand Down Expand Up @@ -220,12 +210,7 @@ func setResponseTypes(client *oauth2client.Client, responseTypesSupported []stri

func setTokenEndpointAuthMethod(client *oauth2client.Client, methodsSupported []string, method string) error {
if method == "" {
if len(methodsSupported) == 0 || lo.Contains(methodsSupported, defaultTokenEndpointAuthMethod) {
client.TokenEndpointAuthMethod = defaultTokenEndpointAuthMethod
} else {
client.TokenEndpointAuthMethod = methodsSupported[0]
}

client.TokenEndpointAuthMethod = oauth2client.TokenEndpointAuthMethodClientSecretBasic
return nil
}

Expand Down Expand Up @@ -300,18 +285,6 @@ func validateClient(client *oauth2client.Client) error {
}
}

// validate relationship between Grant Types and Response Types
// https://datatracker.ietf.org/doc/html/rfc7591#section-2.1
if lo.Contains(client.GrantTypes, "authorization_code") && !lo.Contains(client.ResponseTypes, "code") {
return InvalidClientMetadataError("response_types",
fmt.Errorf("authorization_code grant type requires code response type"))
}

if lo.Contains(client.GrantTypes, "implicit") && !lo.Contains(client.ResponseTypes, "token") {
return InvalidClientMetadataError("response_types",
fmt.Errorf("implicit grant type requires token response type"))
}

return nil
}

Expand Down
85 changes: 10 additions & 75 deletions pkg/service/clientmanager/client_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ func TestManager_Create(t *testing.T) {
Return(
&profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
ScopesSupported: []string{"foo", "bar"},
GrantTypesSupported: []string{"authorization_code"},
ResponseTypesSupported: []string{"code"},
TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"},
EnableDynamicClientRegistration: true,
ScopesSupported: []string{"foo", "bar"},
EnableDynamicClientRegistration: true,
},
}, nil)

mockStore.EXPECT().InsertClient(gomock.Any(), gomock.Any()).Return(uuid.New().String(), nil)

data = &clientmanager.ClientMetadata{
Scope: "foo",
RedirectURIs: []string{"https://example.com/redirect"},
Scope: "foo",
GrantTypes: []string{"authorization_code"},
ResponseTypes: []string{"code"},
TokenEndpointAuthMethod: "client_secret_basic",
RedirectURIs: []string{"https://example.com/redirect"},
}
},
check: func(t *testing.T, client *oauth2client.Client, err error) {
Expand Down Expand Up @@ -125,7 +125,6 @@ func TestManager_Create(t *testing.T) {
Return(
&profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
GrantTypesSupported: []string{"authorization_code"},
EnableDynamicClientRegistration: true,
},
}, nil)
Expand All @@ -152,7 +151,6 @@ func TestManager_Create(t *testing.T) {
Return(
&profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
ResponseTypesSupported: []string{"code"},
EnableDynamicClientRegistration: true,
},
}, nil)
Expand All @@ -179,15 +177,14 @@ func TestManager_Create(t *testing.T) {
Return(
&profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"},
EnableDynamicClientRegistration: true,
EnableDynamicClientRegistration: true,
},
}, nil)

mockStore.EXPECT().InsertClient(gomock.Any(), gomock.Any()).Times(0)

data = &clientmanager.ClientMetadata{
TokenEndpointAuthMethod: "none",
TokenEndpointAuthMethod: "not_supported_auth_method",
}
},
check: func(t *testing.T, client *oauth2client.Client, err error) {
Expand All @@ -196,7 +193,7 @@ func TestManager_Create(t *testing.T) {
require.ErrorAs(t, err, &regErr)
require.Equal(t, clientmanager.ErrCodeInvalidClientMetadata, regErr.Code)
require.Equal(t, "token_endpoint_auth_method", regErr.InvalidValue)
require.Equal(t, "token endpoint auth method none not supported", regErr.Error())
require.Equal(t, "token endpoint auth method not_supported_auth_method not supported", regErr.Error())
},
},
{
Expand Down Expand Up @@ -291,7 +288,6 @@ func TestManager_Create(t *testing.T) {
Return(
&profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
GrantTypesSupported: []string{"authorization_code"},
EnableDynamicClientRegistration: true,
},
}, nil)
Expand All @@ -311,65 +307,6 @@ func TestManager_Create(t *testing.T) {
require.Equal(t, "redirect_uris must be set for authorization_code grant type", regErr.Error())
},
},
{
name: "authorization_code grant type requires code response type error",
setup: func() {
mockProfileSvc.EXPECT().GetProfile(gomock.Any(), gomock.Any()).
Return(
&profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
GrantTypesSupported: []string{"authorization_code", "implicit"},
ResponseTypesSupported: []string{"code", "token"},
EnableDynamicClientRegistration: true,
},
}, nil)

mockStore.EXPECT().InsertClient(gomock.Any(), gomock.Any()).Times(0)

data = &clientmanager.ClientMetadata{
GrantTypes: []string{"authorization_code"},
ResponseTypes: []string{"token"},
RedirectURIs: []string{"https://example.com/redirect"},
}
},
check: func(t *testing.T, client *oauth2client.Client, err error) {
var regErr *clientmanager.RegistrationError

require.ErrorAs(t, err, &regErr)
require.Equal(t, clientmanager.ErrCodeInvalidClientMetadata, regErr.Code)
require.Equal(t, "response_types", regErr.InvalidValue)
require.Equal(t, "authorization_code grant type requires code response type", regErr.Error())
},
},
{
name: "implicit grant type requires token response type error",
setup: func() {
mockProfileSvc.EXPECT().GetProfile(gomock.Any(), gomock.Any()).
Return(
&profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
GrantTypesSupported: []string{"authorization_code", "implicit"},
ResponseTypesSupported: []string{"code", "token"},
EnableDynamicClientRegistration: true,
},
}, nil)

mockStore.EXPECT().InsertClient(gomock.Any(), gomock.Any()).Times(0)

data = &clientmanager.ClientMetadata{
GrantTypes: []string{"implicit"},
ResponseTypes: []string{"code"},
}
},
check: func(t *testing.T, client *oauth2client.Client, err error) {
var regErr *clientmanager.RegistrationError

require.ErrorAs(t, err, &regErr)
require.Equal(t, clientmanager.ErrCodeInvalidClientMetadata, regErr.Code)
require.Equal(t, "response_types", regErr.InvalidValue)
require.Equal(t, "implicit grant type requires token response type", regErr.Error())
},
},
{
name: "invalid redirect uri",
setup: func() {
Expand Down Expand Up @@ -456,8 +393,6 @@ func TestManager_Create(t *testing.T) {
&profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
ScopesSupported: []string{"foo", "bar"},
GrantTypesSupported: []string{"authorization_code"},
ResponseTypesSupported: []string{"code"},
EnableDynamicClientRegistration: true,
},
}, nil)
Expand Down

0 comments on commit 693c498

Please sign in to comment.