Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PKCE verification support #128

Merged
merged 3 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,27 @@ config.omniauth :openid_connect, {

### Options Overview

| Field | Description | Required | Default | Example/Options |
|------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|----------------------------|-----------------------------------------------------|
| name | Arbitrary string to identify connection and identify it from other openid_connect providers | no | String: openid_connect | :my_idp |
| issuer | Root url for the authorization server | yes | | https://myprovider.com |
| discovery | Should OpenID discovery be used. This is recommended if the IDP provides a discovery endpoint. See client config for how to manually enter discovered values. | no | false | one of: true, false |
| client_auth_method | Which authentication method to use to authenticate your app with the authorization server | no | Sym: basic | "basic", "jwks" |
| scope | Which OpenID scopes to include (:openid is always required) | no | Array<sym> [:openid] | [:openid, :profile, :email] |
| response_type | Which OAuth2 response type to use with the authorization request | no | String: code | one of: 'code', 'id_token' |
| state | A value to be used for the OAuth2 state parameter on the authorization request. Can be a proc that generates a string. | no | Random 16 character string | Proc.new { SecureRandom.hex(32) } |
| response_mode | The response mode per [spec](https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html) | no | nil | one of: :query, :fragment, :form_post, :web_message |
| display | An optional parameter to the authorization request to determine how the authorization and consent page | no | nil | one of: :page, :popup, :touch, :wap |
| prompt | An optional parameter to the authrization request to determine what pages the user will be shown | no | nil | one of: :none, :login, :consent, :select_account |
| send_scope_to_token_endpoint | Should the scope parameter be sent to the authorization token endpoint? | no | true | one of: true, false |
| post_logout_redirect_uri | The logout redirect uri to use per the [session management draft](https://openid.net/specs/openid-connect-session-1_0.html) | no | empty | https://myapp.com/logout/callback |
| uid_field | The field of the user info response to be used as a unique id | no | 'sub' | "sub", "preferred_username" |
| extra_authorize_params | A hash of extra fixed parameters that will be merged to the authorization request | no | Hash | {"tenant" => "common"} |
| allow_authorize_params | A list of allowed dynamic parameters that will be merged to the authorization request | no | Array | [:screen_name] |
| client_options | A hash of client options detailed in its own section | yes | | |
| Field | Description | Required | Default | Example/Options |
|------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------------------------------|-----------------------------------------------------|
| name | Arbitrary string to identify connection and identify it from other openid_connect providers | no | String: openid_connect | :my_idp |
| issuer | Root url for the authorization server | yes | | https://myprovider.com |
| discovery | Should OpenID discovery be used. This is recommended if the IDP provides a discovery endpoint. See client config for how to manually enter discovered values. | no | false | one of: true, false |
| client_auth_method | Which authentication method to use to authenticate your app with the authorization server | no | Sym: basic | "basic", "jwks" |
| scope | Which OpenID scopes to include (:openid is always required) | no | Array<sym> [:openid] | [:openid, :profile, :email] |
| response_type | Which OAuth2 response type to use with the authorization request | no | String: code | one of: 'code', 'id_token' |
| state | A value to be used for the OAuth2 state parameter on the authorization request. Can be a proc that generates a string. | no | Random 16 character string | Proc.new { SecureRandom.hex(32) } |
| response_mode | The response mode per [spec](https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html) | no | nil | one of: :query, :fragment, :form_post, :web_message |
| display | An optional parameter to the authorization request to determine how the authorization and consent page | no | nil | one of: :page, :popup, :touch, :wap |
| prompt | An optional parameter to the authrization request to determine what pages the user will be shown | no | nil | one of: :none, :login, :consent, :select_account |
| send_scope_to_token_endpoint | Should the scope parameter be sent to the authorization token endpoint? | no | true | one of: true, false |
| post_logout_redirect_uri | The logout redirect uri to use per the [session management draft](https://openid.net/specs/openid-connect-session-1_0.html) | no | empty | https://myapp.com/logout/callback |
| uid_field | The field of the user info response to be used as a unique id | no | 'sub' | "sub", "preferred_username" |
| extra_authorize_params | A hash of extra fixed parameters that will be merged to the authorization request | no | Hash | {"tenant" => "common"} |
| allow_authorize_params | A list of allowed dynamic parameters that will be merged to the authorization request | no | Array | [:screen_name] |
| pkce | Enable [PKCE flow](https://oauth.net/2/pkce/) | no | false | one of: true, false |
| pkce_verifier | Specify a custom PKCE verifier code. | no | A random 128-char string | Proc.new { SecureRandom.hex(64) } |
| pkce_options | Specify a custom implementation of the PKCE code challenge/method. | no | SHA256(code_challenge) in hex | Proc to customise the code challenge generation |
| client_options | A hash of client options detailed in its own section | yes | | |

### Client Config Options

Expand Down Expand Up @@ -94,7 +97,7 @@ These are the configuration options for the client_options hash of the configura

* `response_type` tells the authorization server which grant type the application wants to use,
currently, only `:code` (Authorization Code grant) and `:id_token` (Implicit grant) are valid.
* If you want to pass `state` paramete by yourself. You can set Proc Object.
* If you want to pass `state` parameter by yourself. You can set Proc Object.
e.g. `state: Proc.new { SecureRandom.hex(32) }`
* `nonce` is optional. If don't want to pass "nonce" parameter to provider, You should specify
`false` to `send_nonce` option. (default true)
Expand All @@ -116,6 +119,11 @@ These are the configuration options for the client_options hash of the configura
property can be used to add the attribute to the token request. Initial value is `true`, which means that the
scope attribute is included by default.

### Additional notes
* In some cases, you may want to go straight to the callback phase - e.g. when requested by a stateless client, like a mobile app.
In such example, the session is empty, so you have to forward certain parameters received from the client.
Currently supported one is `code_verifier` - simply provide it as the `/callback` request parameter.

For the full low down on OpenID Connect, please check out
[the spec](http://openid.net/specs/openid-connect-core-1_0.html).

Expand Down
32 changes: 29 additions & 3 deletions lib/omniauth/strategies/openid_connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ class OpenIDConnect
option :extra_authorize_params, {}
option :allow_authorize_params, []
option :uid_field, 'sub'
option :pkce, false
option :pkce_verifier, nil
option :pkce_options, {
code_challenge: proc { |verifier|
Base64.urlsafe_encode64(Digest::SHA2.digest(verifier), padding: false)
stanhu marked this conversation as resolved.
Show resolved Hide resolved
},
code_challenge_method: 'S256',
}

def uid
user_info.raw_attributes[options.uid_field.to_sym] || user_info.sub
Expand Down Expand Up @@ -178,6 +186,13 @@ def authorize_uri
opts[key] = request.params[key.to_s] unless opts.key?(key)
end

if options.pkce
verifier = options.pkce_verifier ? options.pkce_verifier.call : SecureRandom.hex(64)

opts.merge!(pkce_authorize_params(verifier))
session['omniauth.pkce.verifier'] = verifier
end

client.authorization_uri(opts.reject { |_k, v| v.nil? })
end

Expand All @@ -187,6 +202,14 @@ def public_key
key_or_secret || config.jwks
end

def pkce_authorize_params(verifier)
# NOTE: see https://tools.ietf.org/html/rfc7636#appendix-A
{
code_challenge: options.pkce_options[:code_challenge].call(verifier),
code_challenge_method: options.pkce_options[:code_challenge_method],
}
end

private

def issuer
Expand Down Expand Up @@ -220,11 +243,14 @@ def user_info
def access_token
return @access_token if @access_token

@access_token = client.access_token!(
token_request_params = {
scope: (options.scope if options.send_scope_to_token_endpoint),
client_auth_method: options.client_auth_method
)
client_auth_method: options.client_auth_method,
}

token_request_params[:code_verifier] = params['code_verifier'] || session.delete('omniauth.pkce.verifier') if options.pkce

@access_token = client.access_token!(token_request_params)
verify_id_token!(@access_token.id_token) if configured_response_type == 'code'

@access_token
Expand Down
35 changes: 35 additions & 0 deletions test/lib/omniauth/strategies/openid_connect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,41 @@ def test_id_token_auth_hash
assert auth_hash.key?('extra')
assert auth_hash['extra'].key?('raw_info')
end

def test_option_pkce
strategy.options.client_options[:host] = 'example.com'

# test pkce disabled
strategy.options.pkce = false

assert((strategy.authorize_uri !~ /code_challenge=/), 'URI must not contain code challenge param')
assert((strategy.authorize_uri !~ /code_challenge_method=/), 'URI must not contain code challenge method param')

# test pkce enabled with default opts
strategy.options.pkce = true

assert(strategy.authorize_uri =~ /code_challenge=/, 'URI must contain code challenge param')
assert(strategy.authorize_uri =~ /code_challenge_method=/, 'URI must contain code challenge method param')

# test pkce with custom verifier code
strategy.options.pkce_verifier = proc { 'dummy_verifier' }
code_challenge_value = Base64.urlsafe_encode64(
Digest::SHA2.digest(strategy.options.pkce_verifier.call),
padding: false
)

assert(strategy.authorize_uri =~ /#{Regexp.quote(code_challenge_value)}/, 'URI must contain code challenge value')

# test pkce with custom options and plain text code
strategy.options.pkce_options =
{
code_challenge: proc { |verifier| verifier },
code_challenge_method: 'plain',
}

assert(strategy.authorize_uri =~ /#{Regexp.quote(strategy.options.pkce_verifier.call)}/,
'URI must contain code challenge value')
end
end
end
end