-
-
Notifications
You must be signed in to change notification settings - Fork 389
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
switch to basic auth for API access #1545
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from a technical pov.
But why do you prefer basic auth instead of dedicated header or bearer token for api auth?
src/main/java/de/rwth/idsg/steve/config/ApiAuthenticationManager.java
Outdated
Show resolved
Hide resolved
pls see #1540 (comment) |
@@ -99,8 +113,19 @@ public void commence(HttpServletRequest request, | |||
response.getWriter().print(jacksonObjectMapper.writeValueAsString(apiResponse)); | |||
} | |||
|
|||
private UserDetails getFromCacheOrDatabase(String username) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you set the cache logic here instead of webUserService.loadUserByUsernameForApi
?
I don't know if it is a choice to not use it but Spring has its own way of doing caches: https://spring.io/guides/gs/caching
And you can still use guava under to wood if you want it: https://docs.spring.io/spring-framework/docs/4.2.x/javadoc-api/org/springframework/cache/guava/GuavaCacheManager.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@goekay Just for my technical knowledge because I didn't use them before: why do you use guava directly and not hidden behind spring cache?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a combination of multiple reasons actually (disclaimer: i have been using both of them for a long time)
- stylistic preference if it is just about a localized cache usage, instead of something big or application-wide
- tighter control i can have with guava. this does not matter when using GuavaCacheManager, since the same can be done with that... but then, if you control guava like this, why introduce spring magic? which brings me to my next point.
- absence of multiple spring layers, abstractions, which can lead to weird misbehaviour and gotchas
- to be consistent with the codebase: steve has this direct usage of Guava at some other places, there is no usage of Spring cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the answer! 👍
why introduce spring magic?
That could be true for almost every spring import ;)
@juherr if you have no objections or no more comments, i want to merge this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
hey @faculoyarte, the user you are showing on the screenshot is the end user, i.e. the customer that has an EV and RFID card and wants to use the stations. the user we added in this PR is the web user, i.e. the operations person that manages stations, someone that belongs to a CPO maybe. this is the person that has access to steve's web ui to do things. the web user gets an api_password with this PR. therefore, these two things are disconnected. there is another PR that will make it available to update/change properties of a web user. therefore, currently the only way to do is to directly modify database tables. |
Perfect, thanks. @goekay |
* switch to basic auth for API access * PR feedback * add cache for API users * PR feedback * start setting/updating api_password * refactor: undo moveApiTokenFromConfigToDatabase prep
No description provided.