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.

Sign up for GitHub

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

OIDC post endpoint #214

Closed
wants to merge 1 commit into from
Closed

OIDC post endpoint #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 12 additions & 1 deletion features/authn.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Feature: Authenticate with Conjur
"""
Then the JSON should have "payload"

Scenario: Authenticate with OIDC code requesting unparsed result
Scenario: Authenticate with OIDC code requesting unparsed result via GET method
When I retrieve the provider details for OIDC authenticator "keycloak"
And I retrieve auth info for the OIDC provider with username: "alice" and password: "alice"
And I run the code:
Expand All @@ -23,3 +23,14 @@ Feature: Authenticate with Conjur
"""
Then the response body contains: "payload"
And the response includes headers

Scenario: Authenticate with OIDC code requesting unparsed result via POST method
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two scenarios relying on the updated Conjur OIDC API currently fail in CI since the functionality has yet to be merged. The tests pass when run locally against a Conjur build containing those updates.

When I retrieve the provider details for OIDC authenticator "keycloak"
And I retrieve auth info for the OIDC provider with username: "alice" and password: "alice"
And I run the code:
"""
$conjur.authenticator_enable "authn-oidc", "keycloak"
Conjur::API.authenticator_authenticate_post("authn-oidc", "keycloak", options: @auth_body)
"""
Then the response body contains: "payload"
And the response includes headers
20 changes: 18 additions & 2 deletions lib/conjur/api/authn.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def login username, password, account: Conjur.configuration.account
# @param [Hash] params Additional params to send to authenticator
# @return [String] A JSON formatted authentication token.
def authenticator_authenticate authenticator, service_id, account: Conjur.configuration.account, options: {}
JSON.parse authenticator_authenticate_get authenticator, service_id, account: account, options: options
JSON.parse authenticator_authenticate_post authenticator, service_id, account: account, options: options
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the default behavior of authenticator_authenticate be to use the POST method?

If yes - should the GET routing be removed entirely?

Copy link
Contributor

@john-odonnell john-odonnell Nov 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably. Optimally, we would use the endpoint that is credential-agnostic for code and refresh token exchanges.

Through our progress so far on the refresh token feature branch, we've maintained the GET behavior for backwards-compatibility. Maybe we should consider maintaining it here, too, until we officially remove it.

end

# Authenticates using a third party authenticator like authn-oidc via GET request.
Expand All @@ -78,7 +78,23 @@ def authenticator_authenticate_get authenticator, service_id, account: Conjur.co
if Conjur.log
Conjur.log << "Authenticating to account #{account} using #{authenticator}/#{service_id}\n"
end
url_for(:authenticator_authenticate, account, service_id, authenticator, options).get
url_for(:authenticator_authenticate_get, account, service_id, authenticator, options).get
end

# Authenticates using a third party authenticator like authn-oidc via POST request.
# It will return an response object containing access/refresh token data.
#
# @param [String] authenticator
# @param [String] service_id
# @param [String] account The organization account.
# @param [Hash] params Additional params to send to authenticator
# @return [RestClient::Response] Response object
def authenticator_authenticate_post authenticator, service_id, account: Conjur.configuration.account, options: {}
if Conjur.log
Conjur.log << "Authenticating to account #{account} using #{authenticator}/#{service_id}\n"
end
encoded_params = URI.encode_www_form(options)
url_for(:authenticator_authenticate_post, account, service_id, authenticator).post(encoded_params, content_type: 'application/www-url-form-encoded')
end

# Exchanges Conjur the API key (refresh token) for an access token. The access token can
Expand Down
9 changes: 8 additions & 1 deletion lib/conjur/api/router/v5.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,20 @@ def authn_authenticate account, username
)[fully_escape account][fully_escape username]['authenticate']
end

def authenticator_authenticate(account, service_id, authenticator, options)
def authenticator_authenticate_get(account, service_id, authenticator, options)
RestClient::Resource.new(
Conjur.configuration.core_url,
Conjur.configuration.rest_client_options
)[fully_escape authenticator][fully_escape service_id][fully_escape account]['authenticate'][options_querystring options]
end

def authenticator_authenticate_post(account, service_id, authenticator)
RestClient::Resource.new(
Conjur.configuration.core_url,
Conjur.configuration.rest_client_options
)[fully_escape authenticator][fully_escape service_id][fully_escape account]['authenticate']
end

def authenticator account, authenticator, service_id, credentials
RestClient::Resource.new(
Conjur.configuration.core_url,
Expand Down