djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove the --use-pkce flag and just always use it.

Browse Source 
Based on the spec, it seems like it's required that OAuth2 servers which do not support PKCE should just ignore the parameters, so this should always work.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
This commit is contained in:
Matt Moyer 2020-10-06 16:10:20 -05:00
parent a13d7ec5a1
commit ce49d8bd7b
No known key found for this signature in database
GPG Key ID: EAE88AD172C5AE2D
2 changed files with 22 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
cmd/pinniped/cmd/login_oidc.go
View File

@ -31,7 +31,6 @@ type oidcLoginParams struct {
listenPort uint16
scopes []string
skipBrowser bool
usePKCE bool
// These parameters capture dependencies that we want to mock during testing.
generateState func() (state.State, error)
@ -52,7 +51,6 @@ func (o *oidcLoginParams) cmd() *cobra.Command {
cmd.Flags().Uint16Var(&o.listenPort, "listen-port", 48095, "TCP port for localhost listener (authorization code flow only).")
cmd.Flags().StringSliceVar(&o.scopes, "scopes", []string{"offline_access", "openid", "email", "profile"}, "OIDC scopes to request during login.")
cmd.Flags().BoolVar(&o.skipBrowser, "skip-browser", false, "Skip opening the browser (just print the URL).")
cmd.Flags().BoolVar(&o.usePKCE, "use-pkce", true, "Use Proof Key for Code Exchange (RFC 7636) during login.")
mustMarkRequired(&cmd, "issuer", "client-id")
return &cmd
}
@ -77,14 +75,16 @@ func (o *oidcLoginParams) runE(cmd *cobra.Command, args []string) error {
return fmt.Errorf("could not generate OIDC state parameter: %w", err)
}
var pkceCode pkce.Code
if o.usePKCE {
pkceCode, err = o.generatePKCE()
if err != nil {
return fmt.Errorf("could not generate OIDC PKCE parameter: %w", err)
}
authCodeOptions = append(authCodeOptions, pkceCode.Challenge(), pkceCode.Method())
// We can always use PKCE (RFC 7636) because the server should always ignore the parameters if it doesn't
// understand them. Per https://tools.ietf.org/html/rfc7636#section-5:
// As the OAuth 2.0 [RFC6749] server responses are unchanged by this specification, client implementations of
// this specification do not need to know if the server has implemented this specification or not and SHOULD
// send the additional parameters as defined in Section 4 to all servers.
pkceCode, err := o.generatePKCE()
if err != nil {
return fmt.Errorf("could not generate OIDC PKCE parameter: %w", err)
}
authCodeOptions = append(authCodeOptions, pkceCode.Challenge(), pkceCode.Method())
// If --skip-browser was passed, override the default browser open function with a Printf() call.
openURL := o.openURL

23
cmd/pinniped/cmd/login_oidc_test.go
View File

@ -46,7 +46,6 @@ func TestLoginOIDCCommand(t *testing.T) {
--listen-port uint16 TCP port for localhost listener (authorization code flow only). (default 48095)
--scopes strings OIDC scopes to request during login. (default [offline_access,openid,email,profile])
--skip-browser Skip opening the browser (just print the URL).
--use-pkce Use Proof Key for Code Exchange (RFC 7636) during login. (default true)
`),
},
{
@ -142,7 +141,6 @@ func TestOIDCLoginRunE(t *testing.T) {
params: oidcLoginParams{
issuer: validServer.URL,
generateState: func() (state.State, error) { return "test-state", nil },
usePKCE: true,
generatePKCE: func() (pkce.Code, error) { return "", fmt.Errorf("some error generating a PKCE code") },
},
wantError: "could not generate OIDC PKCE parameter: some error generating a PKCE code",
@ -152,19 +150,18 @@ func TestOIDCLoginRunE(t *testing.T) {
params: oidcLoginParams{
issuer: validServer.URL,
generateState: func() (state.State, error) { return "test-state", nil },
usePKCE: true,
generatePKCE: func() (pkce.Code, error) { return "test-pkce", nil },
openURL: func(_ string) error { return fmt.Errorf("some browser open error") },
},
wantError: "could not open browser (run again with --skip-browser?): some browser open error",
},
{
name: "success without PKCE",
name: "success",
params: oidcLoginParams{
issuer: validServer.URL,
clientID: "test-client-id",
generateState: func() (state.State, error) { return "test-state", nil },
usePKCE: false,
generatePKCE: func() (pkce.Code, error) { return "test-pkce", nil },
listenPort: 12345,
skipBrowser: true,
},
@ -172,12 +169,18 @@ func TestOIDCLoginRunE(t *testing.T) {
require.Equal(t, validServerURL.Host, actual.Host)
require.Equal(t, "/auth", actual.Path)
require.Equal(t, "", actual.Fragment)
require.Equal(t, url.Values{
"access_type": []string{"offline"},
"client_id": []string{"test-client-id"},
"redirect_uri": []string{"http://localhost:12345/callback"},
"response_type": []string{"code"},
"state": []string{"test-state"},
"access_type": []string{"offline"},
"client_id": []string{"test-client-id"},
"redirect_uri": []string{"http://localhost:12345/callback"},
"response_type": []string{"code"},
"state": []string{"test-state"},
"code_challenge_method": []string{"S256"},
// This is the PKCE challenge which is calculated as base64(sha256("test-pkce")). For example:
// $ echo -n test-pkce | shasum -a 256 | cut -d" " -f1 | xxd -r -p | base64 | cut -d"=" -f1
// VVaezYqum7reIhoavCHD1n2d+piN3r/mywoYj7fCR7g
"code_challenge": []string{"VVaezYqum7reIhoavCHD1n2d-piN3r_mywoYj7fCR7g"},
}, actual.Query())
},
wantStderr: "Please log in: <URL>\n",