Skip to content
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

Extract tag authorization in a dedicated service #1005

Merged
merged 6 commits into from
Jul 20, 2024

Conversation

juherr
Copy link
Contributor

@juherr juherr commented Dec 15, 2022

The objective of the change is to be able to delegate the authorization responsibility to an outside EMSP service if wanted.

WDYT?

@juherr juherr force-pushed the feature/external-authorization branch 2 times, most recently from 5922f72 to c9333e0 Compare December 18, 2022 12:58
@juherr juherr marked this pull request as ready for review December 18, 2022 13:47
@goekay
Copy link
Member

goekay commented Jul 14, 2024

late feedback:

i think the goal is valuable. couple of suggestion from my side:

  • can we do these without database changes?
  • your changes are only encapsulating the auth part while leaving OcppTagActivityRecord record = ocppTagRepository.getRecord(idTag); as is. i would make this also part of the auth, because in such external cases the record does not necessarily have to be on local db.

@juherr juherr force-pushed the feature/external-authorization branch from c9333e0 to f04d6f1 Compare July 14, 2024 12:24
@juherr
Copy link
Contributor Author

juherr commented Jul 14, 2024

@goekay done as suggested

@goekay
Copy link
Member

goekay commented Jul 20, 2024

@juherr are you willing to do the changes or should i do them after merging?

@juherr
Copy link
Contributor Author

juherr commented Jul 20, 2024

@goekay I'm currently in holidays and without a computer for the next 2 weeks.

You can wait or make the changed.
Fyi, you should be able to push on my branch and fix before the merge.

@goekay goekay self-requested a review July 20, 2024 11:23
reason: allow multiple implementations of the same interface to exist.
therefore, another impl (for calling external EMSP service) and bean
can exist with @primary annotation which can take precedence
@goekay goekay merged commit 6674251 into steve-community:master Jul 20, 2024
22 checks passed
@goekay
Copy link
Member

goekay commented Jul 20, 2024

thanks @juherr !

@juherr juherr deleted the feature/external-authorization branch July 20, 2024 12:04
@juherr
Copy link
Contributor Author

juherr commented Jul 20, 2024

@goekay thanks for the merge. Please review the other pull requests if you find some time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants