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

Docs: Spring security guide #37530

Closed
fedinskiy opened this issue Dec 5, 2023 · 7 comments · Fixed by #37560
Closed

Docs: Spring security guide #37530

fedinskiy opened this issue Dec 5, 2023 · 7 comments · Fixed by #37560
Labels
area/documentation area/security area/spring Issues relating to the Spring integration kind/bug Something isn't working
Milestone

Comments

@fedinskiy
Copy link
Contributor

Describe the bug

I went through "Quarkus Extension for Spring Security API" [1] guide and found several issue:

  1. "GreetingController" sections[2] says, that "the Quarkus Maven plugin automatically generated a controller with the Spring Web". Maven command from the first section[3] does not generate the controller, unless -DnoCode option is removed. If the option is omitted, the generated code looks a bit differently. Same goes for GreetingControllerTest.
  2. We ask user to write tests[4], which never run, since we ask them to check the access manually. We should at least mention, that these tests can be run with mvn clean test or by pressing r while in the DevMode.
  3. Section "test the changes" recommends opening the page in browser and logging in with different credentials. In Firefox(at least) I can not log in again, unless I delete the history or run in private tab. We should probably add a note, which mentions that.
  4. We should either remove the section about native executable[6] (there is nothing new, compared to the native guide[7]) or add an example on running (and not just generating) the executable.
  5. Is section https://quarkus.io/version/main/guides/spring-security#supported-spring-security-functionalities up-to date? Do we still support only two features? Same question goes for Combining expressions[8]
  6. The same section doesn't explain the @Secured annotation and doesn't explain the differences between it and @PreAuthorize annotation. The link to the Spring documentation will be much appreciated here.
  7. The same section. I am not a native English speaker, but the word "functionalities" seems to be out of place here. I presume, that "functionality" (singular) or "annotations" would be a better fit.
  8. Combining expression sections says, that "currently parentheses are not supported", while an example right before it contains "hasAnyRole('view1', 'view2') OR isAnonymous() OR hasRole('test')". I suppose, we should say, that "expressions do not support parentheses for logical operators and are evaluated from left to right"
  9. Currently, conversion table[9] only shows, how to replace @Secured annotation from Spring. We should probably reference this guide[10] to help with replacement of @PreAuthorize annotation
  10. It's probably worth adding a note, that for the #person.name format, class Person has to have getName() method and remove "or field of the parameter" from the description (fields, even public, can not be used for this check)
  11. Example for PersonChecker contains @Override annotation on a method, which is not inherited, it can probably be deleted.

[1] https://quarkus.io/version/main/guides/spring-security
[2] https://quarkus.io/version/main/guides/spring-security#greetingcontroller
[3] https://quarkus.io/version/main/guides/spring-security#creating-the-maven-project
[4] https://quarkus.io/version/main/guides/spring-security#greetingcontrollertest-2
[5] https://quarkus.io/version/main/guides/spring-security#test-the-changes
[6] https://quarkus.io/version/main/guides/spring-security#run-the-application-as-a-native-executable
[7] https://quarkus.io/guides/building-native-image
[8] https://quarkus.io/version/main/guides/spring-security#combining-expressions
[9] https://quarkus.io/version/main/guides/spring-security#conversion-table
[10] https://quarkus.io/guides/security-authorize-web-endpoints-reference

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@fedinskiy fedinskiy added the kind/bug Something isn't working label Dec 5, 2023
@quarkus-bot quarkus-bot bot added area/security area/spring Issues relating to the Spring integration labels Dec 5, 2023
Copy link

quarkus-bot bot commented Dec 5, 2023

/cc @geoand (spring), @sberyozkin (security)

@fedinskiy
Copy link
Contributor Author

area/documentation

@geoand
Copy link
Contributor

geoand commented Dec 5, 2023

@fedinskiy as it seems you already pinpointed the issues, care to open a docs PR fix with the fixes?

@fedinskiy
Copy link
Contributor Author

@geoand I will try, but I need guidance on points 4, 5 and 7

@geoand
Copy link
Contributor

geoand commented Dec 6, 2023

4. -> fine to remove

5. -> still applies

6. -> go ahead and link to Spring docs

@fedinskiy
Copy link
Contributor Author

@geoand 7, not 6.
As I've said, I am not a native speaker, so I do not know, if my wild linguistic guesses are correct :)

@geoand
Copy link
Contributor

geoand commented Dec 6, 2023

Right, let's go with annotations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation area/security area/spring Issues relating to the Spring integration kind/bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

3 participants