-
Notifications
You must be signed in to change notification settings - Fork 0
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
Create annotation to validate role and serialize roles #147
base: develop
Are you sure you want to change the base?
Conversation
f1d9d42
to
073d8e8
Compare
073d8e8
to
d83187b
Compare
26fe416
to
a949fd7
Compare
a949fd7
to
b0bf6c2
Compare
src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt
Outdated
Show resolved
Hide resolved
1977986
to
5123e2b
Compare
b03f3ec
to
ae82402
Compare
ae82402
to
e5213fe
Compare
Check the documentation preview: https://64b094777f7c04596edc78c5--niaefeup-backend-docs.netlify.app |
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.
Great job so far! I have some questions and suggestions
src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/model/PerActivityRole.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/service/activity/AbstractActivityService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/config/security/AuthConfig.kt
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt
Outdated
Show resolved
Hide resolved
I wrote about this feature in https://github.com/NIAEFEUP/website-niaefeup-backend/wiki/Roles-and-Generations#access-control, please update the information once this is finished |
21320d2
to
4434c16
Compare
Check the documentation preview: https://65060abbfbb37b4dc1cf0e8d--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://650b0e56a26a060465039aec--niaefeup-backend-docs.netlify.app |
src/main/kotlin/pt/up/fe/ni/website/backend/model/PerActivityRole.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/repository/ActivityRepository.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/config/security/AuthConfig.kt
Show resolved
Hide resolved
5a4806f
to
edbfacf
Compare
Check the documentation preview: https://66ae04c443ec4d5daff6f5b5--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://66ae0969a22e2a6a3fea5206--niaefeup-backend-docs.netlify.app |
@BrunoRosendo pls review |
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.
Good job! I just have a small comment.
From now on, we can apply the logic to existent endpoints, instead of using the test endpoint
return authentication.authorities.any { | ||
val payload = it.toString().split(":") | ||
payload.size == 2 && payload[0].toLong() == activityId && payload[1].split("-").any { p -> | ||
p == permission.trim().uppercase(Locale.getDefault()) | ||
} |
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.
I understand but I think it could be a bit more readable. Maybe extract some variables?
Check the documentation preview: https://66ca4d1fe850fcfa814462fb--niaefeup-backend-docs.netlify.app |
Closes #81
Review checklist