Closed aeneasr closed 8 years ago
@matteosuppo @boyvinall @ericdouglas @janekolszak do you think the endpoints /warden/authorized
and /warden/allowed
are semantically easy to understand?
/warden/authorized
: Check if a token + scope is ok/warden/allowed
: Check if token + scope + action + resource is okAdditionally those two functionalities should be added:
Do you think those should be merged to one endpoint? Or do you have naming ideas?
Additionally, I want to implement a strict mode which will only return the data if the introspection token audience (from post body) and the authorization token (from header) match to prevent token substitution attacks.
On top of that one API should be able to check for anonymous users on the warden endpoint. This would also need either a dedicated endpoint or maybe some query parameter?
Here's my proposal. In total we have these capabilities:
Warden: Endpoints for resource providers (usually private)
/warden/token/valid
: Check if a token + scope is valid, returns context data (subject, scopes, ...) if valid/warden/token/allowed
: Check if a token + scope is valid and the token's subject is allowed to do something, returns context data (subject, scopes, ...) if valid/warden/allowed
Check if a subject (e.g. anonymous user, service) is allowed to do somethingOAuth2 endpoints (usually public)
oauth2/introspect
Check if a token + scope is valid, using strict mode (implementation of rfc7662)What do you think?
/token/valid and /token allowed are much more understandable than /authorized and /allowed.
I like the 4 endpoints, they sounds very clear. Do you need help implementing them? Next week I should put some efforts in our migration to hydra, so I can help.
Great! Let me know on Gitter when you have time. :)
I think the naming makes sense and I'll implement it in the next days
I think this makes much more sense:
type Firewall interface {
// InspectToken checks if the given token is valid and if the requested scopes are satisfied. Returns
// a context if the token is valid and an error if not.
InspectToken(ctx context.Context, token string, scopes ...string) (*Context, error)
// InspectTokenFromHTTP uses the HTTP request to decide weather a token is valid or not. If not, an error
// is returned.
InspectTokenFromHTTP(ctx context.Context, r *http.Request, scopes ...string) (*Context, error)
// IsAllowed uses policies to return nil if the access request can be fulfilled or an error if not.
IsAllowed(ctx context.Context, accessRequest *ladon.Request) error
}
Do you mean instead of having http endpoints?
No, it's just the client SDK
I should have said "instead of the current client sdk" -sorry for that :)
there will be some more changes. it would be awesome if you could review this
It seems pretty clear to me. There's only a typo in /warden/allowed. It says "This endpoint requires a token, a scope, a resource name, an action name and a context." While it clearly need a subject, not a token.
Right, thanks!
/warden/authorized
should be renamed/warden/allowed
should be renamed