Skip to content

Conversation

@rullzer
Copy link
Member

@rullzer rullzer commented May 15, 2018

This for example will allow rotating the apptoken for oauth

Signed-off-by: Roeland Jago Douma roeland@famdouma.nl

@codecov
Copy link

codecov bot commented May 15, 2018

Codecov Report

Merging #9484 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #9484      +/-   ##
============================================
+ Coverage     51.68%   51.69%   +<.01%     
- Complexity    25720    25721       +1     
============================================
  Files          1641     1641              
  Lines         96441    96443       +2     
  Branches       1393     1393              
============================================
+ Hits          49850    49858       +8     
+ Misses        46591    46585       -6
Impacted Files Coverage Δ Complexity Δ
lib/private/Authentication/Token/DefaultToken.php 83.92% <100%> (+1.23%) 18 <2> (+2) ⬆️
...vate/Authentication/Token/DefaultTokenProvider.php 97.91% <100%> (+3.53%) 28 <2> (+2) ⬆️
lib/private/Files/Cache/Propagator.php 94.93% <0%> (-1.27%) 16% <0%> (ø)
lib/private/Preview/WatcherConnector.php 90.9% <0%> (-0.76%) 5% <0%> (ø)
apps/files_versions/lib/Storage.php 66.06% <0%> (-0.26%) 103% <0%> (ø)
apps/files_trashbin/lib/Expiration.php 91.93% <0%> (+1.61%) 29% <0%> (ø) ⬇️
lib/private/Preview/Watcher.php 77.77% <0%> (+13.49%) 4% <0%> (-3%) ⬇️

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.

Code makes sense 👍

@rullzer rullzer mentioned this pull request May 16, 2018
1 task
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.

Looks good. Found one minor naming issue.

* @param string $newTokenId
* @return IToken
*/
public function setNewToken(IToken $token, string $oldTokenId, string $newTokenId): IToken;
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to name the method rotate or similar?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes will do. To much token in this indeed


/**
* Get the name of the token
* @return string
Copy link
Member

Choose a reason for hiding this comment

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

nitpick for future PRs: return type annotations are not necessary here 😉

This for example will allow rotating the apptoken for oauth

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer force-pushed the feature/noid/support_rotating_tokens branch from de6812c to aba2559 Compare May 16, 2018 17:27
@rullzer rullzer added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 16, 2018
@rullzer rullzer merged commit 81ab924 into master May 16, 2018
@rullzer rullzer deleted the feature/noid/support_rotating_tokens branch May 16, 2018 18:03
This was referenced May 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish enhancement feature: authentication

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants