add logout endpoint to server #4058
Conversation
|
not sure why travis is not running. |
|
dang, looks like travis is joining the "open source pay up" choir along with dockerhub: |
|
useful article regarding travis new billing model here: |
|
neat stuff and couldn't find any escaping/translation issues :) |
|
thanks Brady, |
|
@bradymiller |
| @@ -126,6 +126,19 @@ | |||
| } elseif ($skipApiAuth) { | |||
| // For endpoints that do not require auth, such as the capability statement | |||
| } else { | |||
| // verify that user tokens haven't been revoked. | |||
| // this is done by verifying the user is trusted with active auth session. | |||
| $isTrusted = $gbl::isTrustedUser($attributes["oauth_client_id"], $attributes["oauth_user_id"]); | |||
There was a problem hiding this comment.
@sjpadgett , Haven't been able to test this, but was just looking through this code and was unable to find the isTrustedUser() function in RestConfig. Am I missing something?
There was a problem hiding this comment.
nice catch sir. I forgot to include file in PR.
I do this alot of late.....
There was a problem hiding this comment.
@sjpadgett ,
tested it with most recent code (including your isTrustedUser function), and this breaks the password grant mechanism since that does not create an entry in oauth_trusted_user (it's too bad travis is dead or it would of actually picked it up on the automated api tests). To support both grant mechanisms, I think we just need to add an active column in oauth_trusted_user and then when logout, it makes it inactive (and when log back in (ie. re-authorize) would make it active). Then here could check for that active status only if there is an entry in oauth_trusted_user.
thoughts?
There was a problem hiding this comment.
btw, I have a bit of extra time that I can work on this today if that would help; i'd also incorporate the global that allows/disallows password grants into this solution (it would give me a bit of a break from some tedious security related stuff i am working on :) ).
There was a problem hiding this comment.
I thought I was creating a trusted user for password. guess not but I wanted to and got lost somewhere.
I also realized i'm using the nonce to verify trusted delete where nonce is optional. Still should work as empty=empty but really need a verification back to the token creation. Therefore i'll have to verify token signature against public key.
The logout is meant to wipe out the session. Same for client and any cookies it generates or session it maintains. Whether the trusted user is there or a flag is set really makes no difference. The issue boils down to the grant type and whether to test or not in dispatcher.
The persist flag I already have is for cases when the refresh token expires or client is not using a refresh then if persist is set, a new signin is not required. Thus the reason for the client to have the ability to log the user out.
I prefer to stay away from flags and keep custom conditionals in dispatch to a minimum. Too me it makes better sense to create a trusted user for any grant type and keep track of its session as well as its grant type. In this way we(site administrators) have the ability to revoke a user to force a new set of tokens.
We need to be thinking ahead for the server admin side of the project to give the practice a way to manage apps and access.
I have to rework the token validation for log out so I can fix this too. For some reason the password grant is an afterthought for me. I think setting it up with a trusted user helps secure it in the end. If anything gives us some control of what the grant is up to.
Sound reasonable to you?
There was a problem hiding this comment.
@sjpadgett , Sounds like a much better plan than mine :)
Provides a logout endpoint that will destroy the trusted users logged in session.