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: security-customization Guide #37569

Open
jedla97 opened this issue Dec 6, 2023 · 4 comments
Open

Docs: security-customization Guide #37569

jedla97 opened this issue Dec 6, 2023 · 4 comments

Comments

@jedla97
Copy link
Contributor

jedla97 commented Dec 6, 2023

Describe the bug

Going through the security-customization guide I found some issues.

  1. HttpAuthenticationMechanism Customization
    - Don't have any imports
    - There is problem with wrong return type of one of the overridden method. As not Uni method is deprecated I think that it should look like this
@Override
    public Uni<HttpCredentialTransport> getCredentialTransport(RoutingContext context) {
        return delegate.getCredentialTransport(context);
    }
  1. Dealing with more than one HttpAuthenticationMechanism
    - Don't have any imports
    - The text talking about priorities but which number for them is better higher or lower. In sense when I select number 1 as in snippet will be used as last mechanism? It would be nice to have one sentence to explain this.
  2. Security Identity Customization
    - Snippet for mtls missing import java.util.Collections
    - Snipet for SecurityIdentitySupplier missing the definition of UserRoleEntity. Can't say what it should contain from this.
  3. Custom Jakarta REST SecurityContext
    - Snippet for mtls missing import java.io.IOException
  4. Disabling Authorization
    - Don't have any imports
    - When testing this I'm not sure if this should "disable/override" @RolesAllowed and @DenyAll and by that allow anyone to access the endpoints.
  5. SunPKCS11
    - The In Quarkus you can achieve the same at the configuration level only without having to modify the code, for example: For me something is off with the words only and without going after each other. Maybe remove only or add comma between them.

For the imports: As I would prefer consistency with the imports, here are some which have the imports and some which missing all imports. It would be nice to have them consistent (at the moment have the imports)

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

@jedla97 jedla97 added the kind/bug Something isn't working label Dec 6, 2023
Copy link

quarkus-bot bot commented Dec 6, 2023

/cc @sberyozkin (security)

@michalvavrik
Copy link
Member

michalvavrik commented Dec 6, 2023

  • When testing this I'm not sure if this should "disable/override" @RolesAllowed and @Denyall and by that allow anyone to access the endpoints.

It does, you can find out by sending HTTP request without credentials that will succeed. Security checks are skipped.

@jedla97
Copy link
Contributor Author

jedla97 commented Dec 7, 2023

It does, you can find out by sending HTTP request without credentials that will succeed. Security checks are skipped.

With this information I adding problem to the list. Method in Disabling Authorization snippet working incorrectly as we setup property disable.authorization to false by default so the method should look like this (see return missing exclamation mark):

@Override
    public boolean isAuthorizationEnabled() {
       return disableAuthorization;
    }

This solution will work if we set disable.authorization=true in application.properties

When testing this I found out that I can't have the DisabledAuthController and quarkus.security.auth.enabled-in-dev-mode=false at the same time. I don't expect anybody using this at the same time but maybe the note about this should bi nice. When using them together I get error jakarta.enterprise.inject.AmbiguousResolutionException: Ambiguous dependencies for type io.quarkus.security.spi.runtime.AuthorizationController and qualifiers [@Default] (there is longer stacktrace but this is just for info).

Edit: when I thinking about the disable by class the property should be defined as true and not change the method. With that it make more sense.

@michalvavrik
Copy link
Member

When testing this I found out that I can't have the DisabledAuthController and quarkus.security.auth.enabled-in-dev-mode=false at the same time. I don't expect anybody using this at the same time but maybe the note about this should bi nice. When using them together I get error jakarta.enterprise.inject.AmbiguousResolutionException: Ambiguous dependencies for type io.quarkus.security.spi.runtime.AuthorizationController and qualifiers [@default] (there is longer stacktrace but this is just for info).

Just tweak priority - io.quarkus.security.spi.runtime.DevModeDisabledAuthorizationController has @Priority(Interceptor.Priority.LIBRARY_AFTER) so if you want to use both (which makes no sense), just define higher priority. IMO not a bug.

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

No branches or pull requests

2 participants