feat(casbin): let the caller decide the method of user identification #41
Conversation
Codecov Report
@@ Coverage Diff @@
## master #41 +/- ##
==========================================
+ Coverage 50.00% 50.60% +0.60%
==========================================
Files 5 5
Lines 326 330 +4
==========================================
+ Hits 163 167 +4
Misses 153 153
Partials 10 10
Continue to review full report at Codecov.
|
|
Very nice @gdemarcsek! Would you mind adding a test for the proposed |
casbin/casbin.go
Outdated
| @@ -109,7 +116,7 @@ func MiddlewareWithConfig(config Config) echo.MiddlewareFunc { | |||
| // GetUserName gets the user name from the request. | |||
| // Currently, only HTTP basic authentication is supported | |||
There was a problem hiding this comment.
Please don't forget to update this comment (thanks to you this method supports "to infinity and beyond!" 😉)
casbin/casbin.go
Outdated
| @@ -109,7 +116,7 @@ func MiddlewareWithConfig(config Config) echo.MiddlewareFunc { | |||
| // GetUserName gets the user name from the request. | |||
| // Currently, only HTTP basic authentication is supported | |||
| func (a *Config) GetUserName(c echo.Context) string { | |||
| username, _, _ := c.Request().BasicAuth() | |||
| username, _ := a.UserGetter(c) | |||
There was a problem hiding this comment.
I'm not sure if ignoring the error it's OK. I know that our current implementation do that, but if this could be an opportunity to improve this code. Could you please change the signature of GetUserName to return also an error and check it on CheckPermission?
|
@pafuent Thanks, sorry, I completely forgot about this, will try to get back to this later this week. |
|
@gdemarcsek Sure, no problem |
This feature lets the caller decide the strategy of identifying the user. The default options remains to be using the username from HTTP basic auth, so it is a non-breaking change that offers more flexibility to users.