search

simplesamlphp / simplesamlphp-module-oidc

A SimpleSAMLphp module for OIDC OP support.
Other
45 stars 22 forks source link

Support single page app browser clients using authorization code and pkce #218

Closed pradtke closed 3 months ago

pradtke commented 4 months ago

Browser based applications have newer implementation recommendations than what appeared in the original OAuth2 and OIDC specs. For a fully browser based public client the recommendation is to use the authorization code flow with pkce. We had to make some patches to this module to get these clients to work in our QA environment. I'll outline the issues we encounter and we can submit a PR once we have time to cleanup the initial work.

Our testing was with swagger UI and Microsoft's authentication library

CORS Support on /token endpoint

The JS client wants to make a POST to the /token/ endpoint to exchange the code. Currently we only set the appropriate headers on the UserInfoController. Recommendation refactor the handleCors method so it can also be used from AccessTokenController

Allow Additional Header

Some JS libraries want to set the header X-Requested-With and fail if not an allowed header.

Recommendation, set 'Access-Control-Allow-Headers' => 'Authorization, X-Requested-With',

Developers want to test with localhost origins

The client form has a regex filter for the allowed origins. This filter excludes https?://localhost and https?://localhost:port variants.

Recommendation, adjust the regex to allow these origins

Response mode fragment

Some JS libraries default to asking, and expecting the code to come back in the fragment. Our authorization code flow only supports returning it in the query. We were able to set the clients to use response_mode=query and work around the issue.

Recommendation: not add fragment support since clients seem to be able to use query mode.

tvdijen commented 4 months ago

RE: Allow Additional Header

https://github.com/simplesamlphp/simplesamlphp/commit/6caa17dc175e6377097290698639c2050abc20a3

It's easy to add headers on the controllers, so this should be an easy fix. These custom headers always take preference over the default ones in SSP's config.php.

The rest of the items are more in @cicnavi 's department :)

cicnavi commented 3 months ago

@pradtke Do you consider this fully resolved with https://github.com/simplesamlphp/simplesamlphp-module-oidc/pull/219 ?

pradtke commented 3 months ago

yes, thanks for the reminder