Skip to content

Commit 40e2ea3

Browse files
cicnavicicnavi
authored
Release refresh token only if offline_access scope is used (#191)
* Introduce offline_access scope * Add offline_access scope to client * Only release refresh token if offline_access scope is used * Validate with some tests * Add config option to always issue refresh token Co-authored-by: Marko Ivančić <[email protected]>
1 parent 36f12eb commit 40e2ea3

File tree

15 files changed

+500
-15
lines changed

15 files changed

+500
-15
lines changed

‎CHANGELOG.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,27 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
66
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
77

8+
## [2.0.4]
9+
### Fixed
10+
- Attempt fix for 'pull access denied for symfonycorp/cli' by @pradtke in #188
11+
- Add Access-Control-Allow-Origin header to responses, if not already present by @cicnavi in #190
12+
13+
14+
## [2.0.3]
15+
### Fixed
16+
- Use InMemory::empty by @pkoenig10 in #186
17+
18+
## [2.0.2] - 2022-07-22
19+
### Fixed
20+
- Correct readme typo for module_oidc.php template path by @dgoosens in #168
21+
- Allow overriding cert+key name/location by @pradtke in #167
22+
- Fix access token timestamps, add issuer by @cicnavi in #174
23+
- Fix PK constraint name for allowed origin table - make it unique by @cicnavi in #173
24+
- Set restart url for authorize commands by @pradtke in #180
25+
- Fix admin-clients link by @Pyrex-FWI in #177
26+
- Logout tokens should have typ header with value 'logout+jwt' by @IlanaRadinsky in #185
27+
- Fail actions on code quality issues by @pradtke in #175
28+
829
## [2.0.1]
930
### Fixed
1031
- Make lib/Store/* available for Symfony DI.

‎UPGRADE.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ not released. If access token was released, user claims will have to be fetched
1919
Note that this option only applies to authorization code flow since implicit flow was not available in version 1.
2020
If you are to use the spec compliant behavior, make sure to warn existing Relying Parties about the change.
2121

22+
Similarly, in version 1, in authorization code flow, refresh token was always released, instead of only
23+
releasing it if the client specifically requested it using 'offline_access' scope. Since changing this
24+
behavior is a potential breaking change for Relying Parties, in version 2 a config option
25+
'alwaysIssueRefreshToken' is introduced to enable OpenID Providers to keep the behavior from version 1
26+
by setting it to 'true'. If 'alwaysIssueRefreshToken' is set to 'false', refresh token will be released
27+
only if it was requested using 'offline_access' scope. If you are to use the spec compliant behavior, make
28+
sure to warn existing Relying Parties about the change. Note that in that case the client must have the
29+
'offline_access' scope registered.
30+
2231
Token endpoint was renamed from '.../access_token.php' to '.../token.php'. This is a potential breaking change
2332
for clients that do not fetch OP configuration from the /.well-known/openid-configuration URI dynamically, but
2433
instead hardcode endpoints in their configuration. You should probably warn existing Relying Parties about this

‎config-templates/module_oidc.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,16 @@
6666

6767
// The claims from the standard scopes should only be put in the ID token when no access token is issued
6868
// For module backwards compatibility you can always include claims in the id token.
69-
// See: https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.4
69+
// @see https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.4
70+
// @deprecated option will be removed in v3.
7071
'alwaysAddClaimsToIdToken' => true,
7172

73+
// Refresh token should only be released if the client requests it using the 'offline_access' scope.
74+
// For module backwards compatibility you can always issue refresh token.
75+
// @see https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
76+
// @deprecated option will be removed in v3.
77+
'alwaysIssueRefreshToken' => true,
78+
7279
// Settings regarding Authentication Processing Filters.
7380
// Note: OIDC authN state array will not contain all of the keys which are available during SAML authN,
7481
// like Service Provider metadata, etc.

‎docker/conformance.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ CREATE TABLE oidc_client (
3434
);
3535
-- Used 'httpd' host for back-channel logout url (https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout)
3636
-- since this is the hostname of conformance server while running in container environment
37-
INSERT INTO oidc_client VALUES('_55a99a1d298da921cb27d700d4604352e51171ebc4','_8967dd97d07cc59db7055e84ac00e79005157c1132','Conformance Client 1',replace('Client 1 for Conformance Testing https://openid.net/certification/connect_op_testing/\n','\n',char(10)),'example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]','https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout');
38-
INSERT INTO oidc_client VALUES('_34efb61060172a11d62101bc804db789f8f9100b0e','_91a4607a1c10ba801268929b961b3f6c067ff82d21','Conformance Client 2','','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email"]',1,1,NULL,NULL,NULL);
37+
INSERT INTO oidc_client VALUES('_55a99a1d298da921cb27d700d4604352e51171ebc4','_8967dd97d07cc59db7055e84ac00e79005157c1132','Conformance Client 1',replace('Client 1 for Conformance Testing https://openid.net/certification/connect_op_testing/\n','\n',char(10)),'example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone","offline_access"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]','https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout');
38+
INSERT INTO oidc_client VALUES('_34efb61060172a11d62101bc804db789f8f9100b0e','_91a4607a1c10ba801268929b961b3f6c067ff82d21','Conformance Client 2','','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","offline_access"]',1,1,NULL,NULL,NULL);
3939
INSERT INTO oidc_client VALUES('_0afb7d18e54b2de8205a93e38ca119e62ee321d031','_944e73bbeec7850d32b68f1b5c780562c955967e4e','Conformance Client 3','Client for client_secret_post','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email"]',1,1,NULL,NULL,NULL);
4040
INSERT INTO oidc_client VALUES('_8957eda35234902ba8343c0cdacac040310f17dfca','_322d16999f9da8b5abc9e9c0c08e853f60f4dc4804','RP-Initiated Logout Client','Client for testing RP-Initiated Logout','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]',NULL);
4141
INSERT INTO oidc_client VALUES('_9fe2f7589ece1b71f5ef75a91847d71bc5125ec2a6','_3c0beb20194179c01d7796c6836f62801e9ed4b368','Back-Channel Logout Client','Client for testing Back-Channel Logout','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]','https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout');

‎docker/ssp/module_oidc.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
],
4747

4848
'alwaysAddClaimsToIdToken' => false,
49+
'alwaysIssueRefreshToken' => false,
50+
4951
// Settings regarding Authentication Processing Filters.
5052
// Note: OIDC authN state array will not contain all of the keys which are available during SAML authN,
5153
// like Service Provider metadata, etc.

‎lib/Factories/Grant/AuthCodeGrantFactory.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use SimpleSAML\Module\oidc\Repositories\AuthCodeRepository;
1919
use SimpleSAML\Module\oidc\Repositories\RefreshTokenRepository;
2020
use SimpleSAML\Module\oidc\Server\Grants\AuthCodeGrant;
21+
use SimpleSAML\Module\oidc\Services\ConfigurationService;
2122
use SimpleSAML\Module\oidc\Utils\Checker\RequestRulesManager;
2223

2324
class AuthCodeGrantFactory
@@ -51,20 +52,24 @@ class AuthCodeGrantFactory
5152
*/
5253
private $requestRulesManager;
5354

55+
private ConfigurationService $configurationService;
56+
5457
public function __construct(
5558
AuthCodeRepository $authCodeRepository,
5659
AccessTokenRepository $accessTokenRepository,
5760
RefreshTokenRepository $refreshTokenRepository,
5861
\DateInterval $refreshTokenDuration,
5962
\DateInterval $authCodeDuration,
60-
RequestRulesManager $requestRulesManager
63+
RequestRulesManager $requestRulesManager,
64+
ConfigurationService $configurationService
6165
) {
6266
$this->authCodeRepository = $authCodeRepository;
6367
$this->accessTokenRepository = $accessTokenRepository;
6468
$this->refreshTokenRepository = $refreshTokenRepository;
6569
$this->refreshTokenDuration = $refreshTokenDuration;
6670
$this->authCodeDuration = $authCodeDuration;
6771
$this->requestRulesManager = $requestRulesManager;
72+
$this->configurationService = $configurationService;
6873
}
6974

7075
public function build(): AuthCodeGrant
@@ -74,7 +79,8 @@ public function build(): AuthCodeGrant
7479
$this->accessTokenRepository,
7580
$this->refreshTokenRepository,
7681
$this->authCodeDuration,
77-
$this->requestRulesManager
82+
$this->requestRulesManager,
83+
$this->configurationService
7884
);
7985
$authCodeGrant->setRefreshTokenTTL($this->refreshTokenDuration);
8086

‎lib/Server/Grants/AuthCodeGrant.php‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\AuthTimeResponseTypeInterface;
3838
use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\NonceResponseTypeInterface;
3939
use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\SessionIdResponseTypeInterface;
40+
use SimpleSAML\Module\oidc\Services\ConfigurationService;
4041
use SimpleSAML\Module\oidc\Utils\Arr;
4142
use SimpleSAML\Module\oidc\Utils\Checker\Interfaces\ResultBagInterface;
4243
use SimpleSAML\Module\oidc\Utils\Checker\RequestRulesManager;
@@ -49,8 +50,10 @@
4950
use SimpleSAML\Module\oidc\Utils\Checker\Rules\RedirectUriRule;
5051
use SimpleSAML\Module\oidc\Utils\Checker\Rules\RequestedClaimsRule;
5152
use SimpleSAML\Module\oidc\Utils\Checker\Rules\RequestParameterRule;
53+
use SimpleSAML\Module\oidc\Utils\Checker\Rules\ScopeOfflineAccessRule;
5254
use SimpleSAML\Module\oidc\Utils\Checker\Rules\ScopeRule;
5355
use SimpleSAML\Module\oidc\Utils\Checker\Rules\StateRule;
56+
use SimpleSAML\Module\oidc\Utils\ScopeHelper;
5457
use stdClass;
5558
use Throwable;
5659

@@ -85,14 +88,17 @@ class AuthCodeGrant extends OAuth2AuthCodeGrant implements
8588

8689
private RequestRulesManager $requestRulesManager;
8790

91+
protected ConfigurationService $configurationService;
92+
8893
protected bool $requireCodeChallengeForPublicClients = true;
8994

9095
public function __construct(
9196
OAuth2AuthCodeRepositoryInterface $authCodeRepository,
9297
AccessTokenRepositoryInterface $accessTokenRepository,
9398
RefreshTokenRepositoryInterface $refreshTokenRepository,
9499
DateInterval $authCodeTTL,
95-
RequestRulesManager $requestRulesManager
100+
RequestRulesManager $requestRulesManager,
101+
ConfigurationService $configurationService
96102
) {
97103
parent::__construct($authCodeRepository, $refreshTokenRepository, $authCodeTTL);
98104

@@ -102,6 +108,7 @@ public function __construct(
102108

103109
$this->authCodeTTL = $authCodeTTL;
104110
$this->requestRulesManager = $requestRulesManager;
111+
$this->configurationService = $configurationService;
105112

106113
if (in_array('sha256', hash_algos(), true)) {
107114
$s256Verifier = new S256Verifier();
@@ -434,12 +441,24 @@ public function respondToAccessTokenRequest(
434441
$responseType->setSessionId($authCodePayload->session_id);
435442
}
436443

437-
// Issue and persist new refresh token if given
438-
$refreshToken = $this->issueRefreshToken($accessToken, $authCodePayload->auth_code_id);
444+
// Refresh token should only be released if the client requests it using the 'offline_access' scope.
445+
// However, for module backwards compatibility we have enabled the deployer to explicitly state that
446+
// the refresh token should always be released.
447+
// @see https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
448+
// TODO in v3 remove this config option and do as per spec.
449+
$alwaysIssueRefreshToken = $this->configurationService
450+
->getOpenIDConnectConfiguration()
451+
->getBoolean('alwaysIssueRefreshToken', true);
452+
453+
// Release refresh token per deployer config or if it is requested by using offline_access scope.
454+
if ($alwaysIssueRefreshToken || ScopeHelper::scopeExists($scopes, 'offline_access')) {
455+
// Issue and persist new refresh token if given
456+
$refreshToken = $this->issueRefreshToken($accessToken, $authCodePayload->auth_code_id);
439457

440-
if ($refreshToken !== null) {
441-
$this->getEmitter()->emit(new RequestEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request));
442-
$responseType->setRefreshToken($refreshToken);
458+
if ($refreshToken !== null) {
459+
$this->getEmitter()->emit(new RequestEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request));
460+
$responseType->setRefreshToken($refreshToken);
461+
}
443462
}
444463

445464
// Revoke used auth code
@@ -506,6 +525,7 @@ public function validateAuthorizationRequestWithCheckerResultBag(
506525
ScopeRule::class,
507526
RequestedClaimsRule::class,
508527
AcrValuesRule::class,
528+
ScopeOfflineAccessRule::class,
509529
];
510530

511531
// Since we have already validated redirect_uri and we have state, make it available for other checkers.

‎lib/Server/ResponseTypes/IdTokenResponse.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ protected function getExtraParams(AccessTokenEntityInterface $accessToken): arra
8383
}
8484
// Per https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.4 certain claims
8585
// should only be added in certain scenarios. Allow deployer to control this.
86+
// TODO in v3 remove this config option and do as per spec.
8687
$addClaimsFromScopes = $this->configurationService
8788
->getOpenIDConnectConfiguration()
8889
->getBoolean('alwaysAddClaimsToIdToken', true);

‎lib/Services/ConfigurationService.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ class ConfigurationService
3131
'openid' => [
3232
'description' => 'openid',
3333
],
34+
'offline_access' => [
35+
'description' => 'offline_access',
36+
],
3437
'profile' => [
3538
'description' => 'profile',
3639
],
@@ -116,7 +119,7 @@ private function validateConfiguration()
116119
* @throws ConfigurationError
117120
*/
118121
function (array $scope, string $name): void {
119-
if (in_array($name, ['openid', 'profile', 'email', 'address', 'phone'], true)) {
122+
if (in_array($name, array_keys(self::$standardClaims), true)) {
120123
throw new ConfigurationError('Can not overwrite protected scope: ' . $name, 'oidc_config.php');
121124
}
122125
if (!array_key_exists('description', $scope)) {

‎lib/Services/Container.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
use SimpleSAML\Module\oidc\Utils\Checker\Rules\RequiredNonceRule;
6767
use SimpleSAML\Module\oidc\Utils\Checker\Rules\RequiredOpenIdScopeRule;
6868
use SimpleSAML\Module\oidc\Utils\Checker\Rules\ResponseTypeRule;
69+
use SimpleSAML\Module\oidc\Utils\Checker\Rules\ScopeOfflineAccessRule;
6970
use SimpleSAML\Module\oidc\Utils\Checker\Rules\ScopeRule;
7071
use SimpleSAML\Module\oidc\Utils\Checker\Rules\StateRule;
7172
use SimpleSAML\Module\oidc\Utils\Checker\Rules\IdTokenHintRule;
@@ -204,6 +205,7 @@ public function __construct()
204205
new PostLogoutRedirectUriRule($clientRepository),
205206
new UiLocalesRule(),
206207
new AcrValuesRule(),
208+
new ScopeOfflineAccessRule($configurationService),
207209
];
208210
$requestRuleManager = new RequestRulesManager($requestRules, $loggerService);
209211
$this->services[RequestRulesManager::class] = $requestRuleManager;
@@ -251,7 +253,8 @@ public function __construct()
251253
$refreshTokenRepository,
252254
$refreshTokenDuration,
253255
$authCodeDuration,
254-
$requestRuleManager
256+
$requestRuleManager,
257+
$configurationService
255258
);
256259
$this->services[AuthCodeGrant::class] = $authCodeGrantFactory->build();
257260

0 commit comments

Comments
 (0)