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

Make OAuth2 authorization code expire #40766

Merged
merged 13 commits into from
Oct 5, 2023
Merged

Conversation

julien-nc
Copy link
Member

Oauth authorization codes now expire 10 minutes after being created.
To know if a row in oauth2_access_tokens is an authorization code or an access token, we keep track of the number of delivered access tokens. If no token was delivered, it means the row still contains an authorization code (in the encrypted_token column).

  • Refuse to deliver an access token from an expired authorization code (and immediately delete the related DB entry)
  • Expired authorization codes are cleaned up every month
  • Fix: A refresh token could be used as an authorization code (with the authorization_code grant type). This is no longer possible
  • A few tests have been implemented

@@ -44,12 +48,18 @@
protected $hashedCode;
/** @var string */
protected $encryptedToken;
/** @var int */
protected $codeCreatedAt;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor Note

Property OCA\OAuth2\Db\AccessToken::$codeCreatedAt is not defined in constructor of OCA\OAuth2\Db\AccessToken or in any methods called in the constructor
/** @var int */
protected $codeCreatedAt;
/** @var int */
protected $tokenCount;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor Note

Property OCA\OAuth2\Db\AccessToken::$tokenCount is not defined in constructor of OCA\OAuth2\Db\AccessToken or in any methods called in the constructor
@julien-nc julien-nc force-pushed the enh/noid/oauth2-code-expiration branch from 2a63356 to 4cb31e1 Compare October 4, 2023 09:54
if (!$table->hasColumn('code_created_at')) {
$table->addColumn('code_created_at', Types::BIGINT, [
'notnull' => true,
'default' => 0,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'default' => 0,
'unsigned' => true,

Default 0 is automatically, unsigned will allow more numbers

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not explicitly setting the default to 0? I didn't know about it, maybe others don't know either.

Copy link
Member

Choose a reason for hiding this comment

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

Also fine 👍🏼

@nickvergessen
Copy link
Member

OpenAPI command also needs to be executed

Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

API changes LGTM

apps/oauth2/lib/Controller/OauthApiController.php Outdated Show resolved Hide resolved
@julien-nc julien-nc force-pushed the enh/noid/oauth2-code-expiration branch from efde8a9 to 3e9e57b Compare October 4, 2023 18:02
@julien-nc julien-nc force-pushed the enh/noid/oauth2-code-expiration branch 2 times, most recently from 5efcbd5 to b2808af Compare October 5, 2023 08:03
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…have expired

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
… (active token)

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…t in authorization state

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc julien-nc force-pushed the enh/noid/oauth2-code-expiration branch from b2808af to d56950a Compare October 5, 2023 12:24
@julien-nc julien-nc merged commit b4fec29 into master Oct 5, 2023
39 checks passed
@julien-nc julien-nc deleted the enh/noid/oauth2-code-expiration branch October 5, 2023 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants