search

cloudfoundry / cf-uaac

Apache License 2.0
41 stars 29 forks source link

feature: authorization_code grant with public client usage #123

Closed strehle closed 1 year ago

strehle commented 1 year ago

Both options together allow the authorization_code grant with public usage (no secret in request)

cf-gitbot commented 1 year ago

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

strehle commented 1 year ago

Test

uaac token authcode get -c login --port 7000

Pre-requisit is latest cf-uaa-lib library which supports PKCE and allows to omit secret

strehle commented 1 year ago

@Tallicia @torsten-sap I add you to define who could to ruby review.

peterhaochen47 commented 1 year ago

https://github.com/cloudfoundry/cf-uaa-lib/pull/90 is merged now. Let's bump cf-uaa-lib so we can test out this feature.

strehle commented 1 year ago

cloudfoundry/cf-uaa-lib#90 is merged now. Let's bump cf-uaa-lib so we can test out this feature.

done

peterhaochen47 commented 1 year ago

@strehle, thanks! Manually tested, working as expected, great!

Question: If as you said here, the PKCE is fully backward compatible (both clients and auth-server have to support it for it to take effect; and the auth request will not fail if one of them does not support it), then what is the purpose of adding the config flag --[no-]pkce from the user perspective? Why not just always try to use PKCE?

strehle commented 1 year ago

@strehle, thanks! Manually tested, working as expected, great!

Question: If as you said here, the PKCE is fully backward compatible (both clients and auth-server have to support it for it to take effect; and the auth request will not fail if one of them does not support it), then what is the purpose of adding the config flag --[no-]pkce from the user perspective? Why not just always try to use PKCE?

ok and yes, removed the options so that it is now only creating the PKCE for authorization_code flow

Tallicia commented 1 year ago

Is this ready to merge or is there anything else that needs to precede merging this?

strehle commented 1 year ago

Is this ready to merge or is there anything else that needs to precede merging this?

could be merged , FYI: @peterhaochen47