-
Notifications
You must be signed in to change notification settings - Fork 382
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.
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
need ACL support for basic authentication middleware #112
Comments
Hi, I like the idea! Unfortunately, this is an API change. This project follows "Semver", breaking API changes are allowed only with major version changes. So, I'll keep this change for the v3.0.0. BTW, I'm currently planning the v3, the ideas are here: #110 |
thanks, i'll keep an eye on v3 roadmap:) |
Thinking more about this, I think it would be nice to dissociate the two steps:
Authorizator func(userId string, request *Request) bool The will be executed after a successful call to This has the benefit to be backward compatible. But the authorization is still required for all requests. |
Hi Antoine agreed, that's the right approach, authorization will proceed only if the Thanks & Best Regards!
--------ooO--(_)--Ooo-------- On Sun, Jan 4, 2015 at 1:53 PM, Antoine Imbert notifications@github.com
|
@missedone The PR is there: #122 I'll merge that soon. |
@ant0ine that looks good to me, thanks:) |
merged, closing this issue. |
Hi ant0ine
go-json-rest is quite well that help me quickly get my simple restful service working, i added basic auth middleware to the restful interfaces.
now i want to add ACL support so that, for example, only Admin can have POST/PUT/DELET privileges while User can only GET.
but the Authenticator signature:
Authenticator: func(userId string, password string) bool
, only pass the user name and password while i want more context, e.g. Method, URI, etc.so i think we can simple pass one more parameter
request *Request
to it:Authenticator: func(userId string, password string, request *Request) bool
(I copied the auth_basic.go and modify it to fit my ACL need locally), but i think this could be common if people need ACL support.The text was updated successfully, but these errors were encountered: