Add revokeTokens route to Security Controller #1374

Yoann-Abbes · 2019-08-01T15:53:19Z

What does this PR do ?

Add a revokeTokens route to the security controller, with tests and documentation page.

This route allows an administrator to revoke all of a user's tokens.

https://jira.kaliop.net/browse/KZL-507

How should this be manually tested?

unit/functional tests

Aschen · 2019-08-01T15:58:11Z

I'm thinking that we could both have revoke all tokens and revoke one token with this route.
User could provide an array of token that need to be revoked to revoke specific token.
If no array if provided, this route revoke all user tokens.

Also maybe this route could be useful in the auth controller to I think, so currently logued users can revoke their tokens

scottinet · 2019-08-01T16:02:18Z

@Aschen That would mean that an administrator would know what tokens to revoke beforehand and that does not sound practical to me. How do you propose to make that work? 🤔

Aschen · 2019-08-01T16:03:25Z

@Aschen That would mean that an administrator would know what tokens to revoke beforehand and that does not sound practical to me. How do you propose to make that work? thinking

I just thinking about and you're right, we would need a security:listToken before

scottinet · 2019-08-01T16:09:21Z

@Aschen I'm absolutely and completely against any addition to the API allowing tokens to be leaked to the outside.

scottinet · 2019-08-01T16:12:51Z

@Aschen > check out JIRA KZL-509, I described how we could let users manage their sessions, without ever leaking tokens. I think this is the way to go, and it would be easy to add API routes in the security controller to allow an administrator to manage another user's sessions.

codecov · 2019-08-01T16:23:16Z

Codecov Report

Merging #1374 into 1-dev will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##            1-dev    #1374      +/-   ##
==========================================
+ Coverage   93.88%   93.88%   +<.01%     
==========================================
  Files         106      106              
  Lines        7291     7298       +7     
==========================================
+ Hits         6845     6852       +7     
  Misses        446      446

Impacted Files	Coverage Δ
lib/config/httpRoutes.js	`100% <ø> (ø)`	⬆️
lib/api/controllers/securityController.js	`98.34% <100%> (+0.03%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff651b3...2797a9a. Read the comment docs.

codecov · 2019-08-01T16:23:17Z

Codecov Report

Merging #1374 into 1-dev will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##            1-dev    #1374      +/-   ##
==========================================
+ Coverage   93.92%   93.93%   +<.01%     
==========================================
  Files         106      106              
  Lines        7330     7337       +7     
==========================================
+ Hits         6885     6892       +7     
  Misses        445      445

Impacted Files	Coverage Δ
lib/config/httpRoutes.js	`100% <ø> (ø)`	⬆️
lib/api/controllers/securityController.js	`98.38% <100%> (+0.03%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a69c80...ce5da7f. Read the comment docs.

…kens-route

Aschen · 2019-08-02T15:23:00Z

@Aschen > check out JIRA KZL-509, I described how we could let users manage their sessions, without ever leaking tokens. I think this is the way to go, and it would be easy to add API routes in the security controller to allow an administrator to manage another user's sessions.

Yes you're right, I have been thinking about it and it's not possible to have this kind of route.
Eventually the route could list token by another identifier but we would need to add metadata (like ip adress) to allow users to identify legitimate sessions.

And what do you think about a auth:revokeTokens to allow currently logged user to revoke his token ?

lib/config/httpRoutes.js

lib/api/controllers/securityController.js

features/kuzzle.feature

test/api/controllers/securityController/users.test.js

doc/1/api/controllers/security/revoke-tokens/index.md

features/kuzzle.feature

lib/api/controllers/securityController.js

Co-Authored-By: Adrien Maret <amaret93@gmail.com>

…kuzzle into KZL-507-add-revokeTokens-route

Aschen

I'm blocking this PR as long as #1333 is not merged so this code can use the errorManager

lib/api/controllers/securityController.js

Aschen · 2019-08-08T07:25:05Z

lib/api/controllers/securityController.js

+        if (!user) {
+          errorsManager.throw('user_not_found', userId);
+        }
+        return this.kuzzle.repositories.token.deleteByUserId(userId);


The documentation indicate that this route return null but here you return the result of deleteByUserId

Adds a revokeTokens route to the security controller, with tests and documentation page

add revokeTokens rout, tests and doc

2797a9a

alexandrebouthinon changed the title ~~Add revokeTokens rout to Security Controller~~ Add revokeTokens route to Security Controller Aug 1, 2019

Yoann-Abbes self-assigned this Aug 1, 2019

Yoann-Abbes added the changelog:enhancements label Aug 1, 2019

scottinet added changelog:new-features Increase the minor version number and removed changelog:enhancements labels Aug 1, 2019

Merge remote-tracking branch 'origin/1-dev' into KZL-507-add-revokeTo…

e3b2e3e

…kens-route

Merge branch '1-dev' into KZL-507-add-revokeTokens-route

b1f40df

benoitvidis suggested changes Aug 5, 2019

View reviewed changes

lib/config/httpRoutes.js Outdated Show resolved Hide resolved

lib/api/controllers/securityController.js Outdated Show resolved Hide resolved

features/kuzzle.feature Outdated Show resolved Hide resolved

test/api/controllers/securityController/users.test.js Outdated Show resolved Hide resolved

@benoitvidis requested changes

9c41b86

Yoann-Abbes requested a review from benoitvidis August 6, 2019 09:22

Aschen suggested changes Aug 6, 2019

View reviewed changes

Yoann-Abbes and others added 6 commits August 6, 2019 12:00

Update doc/1/api/controllers/security/revoke-tokens/index.md

c4024c9

Co-Authored-By: Adrien Maret <amaret93@gmail.com>

Update doc/1/api/controllers/security/revoke-tokens/index.md

747b4f5

Co-Authored-By: Adrien Maret <amaret93@gmail.com>

Merge branch '1-dev' into KZL-507-add-revokeTokens-route

7106acf

functional test

619c737

Merge branch 'KZL-507-add-revokeTokens-route' of github.com:kuzzleio/…

b884de2

…kuzzle into KZL-507-add-revokeTokens-route

reverse doc change

48a481e

Yoann-Abbes requested a review from Aschen August 6, 2019 11:10

Aschen approved these changes Aug 6, 2019

View reviewed changes

alexandrebouthinon approved these changes Aug 6, 2019

View reviewed changes

Aschen suggested changes Aug 6, 2019

View reviewed changes

benoitvidis approved these changes Aug 7, 2019

View reviewed changes

Aschen suggested changes Aug 7, 2019

View reviewed changes

lib/api/controllers/securityController.js Outdated Show resolved Hide resolved

Yoann-Abbes added 2 commits August 7, 2019 19:00

Merge branch '1-dev' into KZL-507-add-revokeTokens-route

007c4a8

subcode

02d4fd6

Yoann-Abbes requested a review from Aschen August 7, 2019 17:03

Aschen suggested changes Aug 8, 2019

View reviewed changes

revokeTokens return null

ce5da7f

Aschen approved these changes Aug 8, 2019

View reviewed changes

Aschen merged commit a84a659 into 1-dev Aug 8, 2019

Aschen deleted the KZL-507-add-revokeTokens-route branch August 8, 2019 08:49

Aschen restored the KZL-507-add-revokeTokens-route branch August 8, 2019 08:49

Aschen deleted the KZL-507-add-revokeTokens-route branch August 8, 2019 08:49

Yoann-Abbes added a commit that referenced this pull request Aug 20, 2019

Add revokeTokens route to Security Controller (#1374)

fd815ab

Adds a revokeTokens route to the security controller, with tests and documentation page

Aschen mentioned this pull request Sep 9, 2019

Release 1.10.0 #1457

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add revokeTokens route to Security Controller #1374

Add revokeTokens route to Security Controller #1374

Yoann-Abbes commented Aug 1, 2019 •

edited by scottinet

Aschen commented Aug 1, 2019 •

edited

scottinet commented Aug 1, 2019

Aschen commented Aug 1, 2019

scottinet commented Aug 1, 2019 •

edited

scottinet commented Aug 1, 2019

codecov bot commented Aug 1, 2019

codecov bot commented Aug 1, 2019 •

edited

Aschen commented Aug 2, 2019 •

edited

Aschen left a comment

Aschen Aug 8, 2019

Add revokeTokens route to Security Controller #1374

Add revokeTokens route to Security Controller #1374

Conversation

Yoann-Abbes commented Aug 1, 2019 • edited by scottinet

What does this PR do ?

How should this be manually tested?

Aschen commented Aug 1, 2019 • edited

scottinet commented Aug 1, 2019

Aschen commented Aug 1, 2019

scottinet commented Aug 1, 2019 • edited

scottinet commented Aug 1, 2019

codecov bot commented Aug 1, 2019

Codecov Report

codecov bot commented Aug 1, 2019 • edited

Codecov Report

Aschen commented Aug 2, 2019 • edited

Aschen left a comment

Choose a reason for hiding this comment

Aschen Aug 8, 2019

Choose a reason for hiding this comment

Yoann-Abbes commented Aug 1, 2019 •

edited by scottinet

Aschen commented Aug 1, 2019 •

edited

scottinet commented Aug 1, 2019 •

edited

codecov bot commented Aug 1, 2019 •

edited

Aschen commented Aug 2, 2019 •

edited