steve-community / steve

SteVe - OCPP server implementation in Java
GNU General Public License v3.0
782 stars 382 forks source link

Store Web API key in database -> Switch to basic auth #1540

Closed goekay closed 1 month ago

goekay commented 1 month ago

Checklist

Describe the problem you are trying to solve

currently, we are storing web api key in properties file. this is problematic for multiple reasons:

Describe the solution you'd like

since we are moving into database for multi-users and rbac (see #991, #1165 and #1539), i am thinking about adding another column api_token to web_user. this way, each user will have the possibility to access web UI and API. with this approach, we will start associating tokens with web users.

Describe alternative solutions or features you've considered

Additional context

...

goekay commented 1 month ago

during the impl of https://github.com/steve-community/steve/pull/1545 i realized the following.

juherr commented 1 month ago

It is logical to consider using Basic Authentication when treating the api_token as an API password. However, as an API integrator, I have not encountered such a method of API authentication before.

It would be beneficial to explore dedicated stacks specifically designed for this purpose, such as Keycloak and other similar options.

In my view, offering a default implementation that is quick and rudimentary can be helpful, as long as it doesn't limit the option of replacing it with a more secure and improved solution in the future.

goekay commented 1 month ago

However, as an API integrator, I have not encountered such a method of API authentication before.

really? i am integrating various APIs in my day job as well and i see all kinds of flavours: some fixed value in URL (no one should do that), custom header key/value, bearer token, basic auth, etc. the variations are so many, that i find the decision even kind of insignificant (except the value/token in URL. no one should do that). basic auth will create a base64 token out of username:pwd anyways which will be sent over the wire.

switching between auth mechanisms in HTTP calls is this easy:

org.springframework.http.HttpHeaders headers = new HttpHeaders();
// opt 1: current impl in steve
headers.set("STEVE-API-KEY", "webapi key value"); 
// opt 2
headers.setBearerAuth("bearer token");
// opt 3
headers.setBasicAuth("username", "pwd");

my problem with custom header and bearer is that i cannot associate them with a web_user.

edit: we could of course add a pre-step and endpoint to create and get a token for a user/pwd combination... for the real API calls to use that token. but i am trying to avoid this extra step.

juherr commented 1 month ago

I agree that it exist many ways to implement authentication in API, but there is a kind of consensus these days for oauth or JWT. I'm not a security expert, and that's why I'm interested in understanding what are the best practices and where they come from. Do you have some public API examples that are using api_token as you proposed?

I don't get the security problem of having an access token in the webuser table. It can be seen as a denormalized way to avoid the token table. Many examples on the web explain how to implement api_token. For example, https://medium.com/@sallu-salman/implementing-token-based-authentication-in-a-spring-boot-project-dba7811ffcee

If you want to keep it simple (and security is never simple), why not just remove the api_token concept and the user login/password with basicauth? The current implementation is still valid as a simple service token.

goekay commented 1 month ago

Do you have some public API examples that are using api_token as you proposed?

if you google "Basic auth for REST APIs", you will see many many results. here is one from atlassian: https://developer.atlassian.com/cloud/jira/platform/basic-auth-for-rest-apis/

in fact, they refer to their API pwd as "api token" as well:

Generate an API token for Jira using your Atlassian Account.

if you read between the lines, they distinguish between normal pwd and api pwd (i.e. api token), as well. this is also my proposition.

If you want to keep it simple (and security is never simple), why not just remove the api_token concept and the user login/password with basicauth?

see my comment from above:

api_token in database actually represents an "api password". this is interesting, and allows an admin/manager to set different passwords for UI and API. both can be set to the same value. i see this as an added expressiveness. is this an overkill?

so, i actually thought about this, but then realized: if people want to use this way they can set both columns (password and api_token) to the same value. the solution i am providing can do that but also more (different values).

juherr commented 1 month ago

You got me, you found someone using the same solution. That doesn't mean it is the recommended one or the current state of the art.

BTW, I think it is an overkill if you want something simple, and may not meet the desired level of perfection in terms of elegance.

goekay commented 1 month ago

That doesn't mean it is the recommended one or the current state of the art.

you wanted examples, i gave an example. but also mentioned there are many similar solutions/posts/results.

juherr commented 1 month ago

You are correct. It seems that Atlassian is the only one that associates "login" and "token" in my first two pages of Google search results when I supposed there is none.

I think the use of the term "token" instead of "password" can be confusing. To avoid this confusion and any debate, a possible solution would be to rename it as "api_password".

goekay commented 1 month ago

please do not get caught by the used terminology. sure, i can rename.

but would you agree with me that "basic auth for rest apis" is indeed common approach now? and that i am not doing something wild?

edit: i think we had a misunderstanding that caused this long discussion. you were confused by the word "api_token" which i was treating as api password in the impl. i even named the variable apiPassword. and i thought, you were against the concept of "basic auth for rest apis".

juherr commented 1 month ago

I don't consider Basic as the best choice, but I have no strong argument against it. It isn't worth debating which one is better because they are both good choices.

In my mind (and maybe only mine) token_api refers to Bearer or specific header.

If the variable is renamed password, there is no possible confusion and Basic will be the best choice in that case.