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

Jump to bottom

[stable13] Improve OAuth #9540

Merged
merged 8 commits into from
May 23, 2018
Merged

[stable13] Improve OAuth #9540

merged 8 commits into from
May 23, 2018

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented May 22, 2018 •
edited
Loading

@rullzer
Allow the rotation of tokens
8fcb7d4 
This for example will allow rotating the apptoken for oauth

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer force-pushed the backport/9517/stable13 branch from 2214708 to 8fcb7d4 Compare May 22, 2018 10:51
@codecov
Copy link

codecov bot commented May 22, 2018 •
edited
Loading

Codecov Report

Merging #9540 into stable13 will increase coverage by 0.01%.
The diff coverage is 62.9%.

@@              Coverage Diff              @@
##             stable13   #9540      +/-   ##
=============================================
+ Coverage       51.38%   51.4%   +0.01%     
- Complexity      25052   25081      +29     
=============================================
  Files            1609    1612       +3     
  Lines           95350   95457     +107     
  Branches         1376    1376              
=============================================
+ Hits            48999   49066      +67     
- Misses          46351   46391      +40
Impacted Files Coverage Δ Complexity Δ
apps/oauth2/composer/composer/autoload_static.php 0% <ø> (ø) 1 <0> (ø) ⬇️
core/Migrations/Version13000Date20180516101403.php 0% <0%> (ø) 2 <2> (?)
version.php 0% <0%> (ø) 0 <0> (ø) ⬇️
...pps/oauth2/composer/composer/autoload_classmap.php 0% <0%> (ø) 0 <0> (ø) ⬇️
apps/oauth2/lib/Migration/SetTokenExpiration.php 0% <0%> (ø) 5 <5> (?)
...vate/Authentication/Token/DefaultTokenProvider.php 98.03% <100%> (+3.65%) 32 <2> (+6) ⬆️
...uthentication/Exceptions/ExpiredTokenException.php 100% <100%> (ø) 2 <2> (?)
lib/private/Authentication/Token/DefaultToken.php 89.58% <100%> (+2.74%) 17 <4> (+4) ⬆️
core/Controller/ClientFlowLoginController.php 79.35% <100%> (ø) 25 <0> (ø) ⬇️
...rivate/Authentication/Token/DefaultTokenMapper.php 100% <100%> (ø) 11 <0> (ø) ⬇️
... and 7 more

@rullzer rullzer force-pushed the backport/9517/stable13 branch 3 times, most recently from a70250d to 710f0dd Compare May 22, 2018 12:20
@rullzer
Certain tokens can expire
46aafe4 
However due to the nature of what we store in the token (encrypted
passwords etc). We can't just delete the tokens because that would make
the oauth refresh useless.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer force-pushed the backport/9517/stable13 branch from 710f0dd to 46aafe4 Compare May 22, 2018 12:26
rullzer added 6 commits May 22, 2018 14:50
@rullzer
Set OAuth token expiration
000cf19 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Fail if the response type is not properly set
a04ea70 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Authenticate the clients on requesting a token
30750e4 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Rotate token
d03265f 
On a refresh token request:
* rorate
* reset expire

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Don't use special chars to avoid confusion
73f8373 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Add tests
3c00270 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer added enhancement 3. to review Waiting for reviews labels May 22, 2018
@rullzer rullzer added this to the Nextcloud 13.0.3 milestone May 22, 2018
ChristophWurst
ChristophWurst approved these changes May 23, 2018
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

LGTM 👍

lib/composer/composer/autoload_static.php
@@ -6,8 +6,14 @@

class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
{
public static $firstCharsPsr4 = array (
'O' => true,
public static $prefixLengthsPsr4 = array (
Copy link
Member

Choose a reason for hiding this comment

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

why was this changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I had to rerun the autoload dump. And I guess there is a new composer version ;)

MorrisJobke
MorrisJobke approved these changes May 23, 2018
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Works 👍

@MorrisJobke MorrisJobke merged commit 2f059d1 into stable13 May 23, 2018
@MorrisJobke MorrisJobke deleted the backport/9517/stable13 branch May 23, 2018 13:42
@MorrisJobke MorrisJobke mentioned this pull request May 31, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@blizzz blizzz Awaiting requested review from blizzz

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Milestone
Nextcloud 13.0.3
Development

Successfully merging this pull request may close these issues.
3 participants
@rullzer @MorrisJobke @ChristophWurst