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

Unable to change unauthenticated response with quarkus-resteasy-reactive #25732

Closed
ivansenic opened this issue May 23, 2022 · 22 comments · Fixed by #28539
Closed

Unable to change unauthenticated response with quarkus-resteasy-reactive #25732

ivansenic opened this issue May 23, 2022 · 22 comments · Fixed by #28539
Assignees
Labels
area/security kind/bug Something isn't working
Milestone

Comments

@ivansenic
Copy link
Contributor

ivansenic commented May 23, 2022

Describe the bug

I tried to add a custom exception mapper for unauthenticated exceptions. Followed the instruction on https://quarkus.io/guides/security-built-in-authentication#how-to-customize-authentication-exception-responses. However, although the exception mapper is defined, it's never being invoked. Switching the proactive auth on and off does not have any impact.

The exception mapper is the same as in the documentation:

import io.quarkus.security.AuthenticationFailedException;

import javax.annotation.Priority;
import javax.ws.rs.Priorities;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;


@Provider
@Priority(Priorities.AUTHENTICATION)
public class AuthenticationFailedExceptionMapper implements ExceptionMapper<AuthenticationFailedException> {

    @Context
    UriInfo uriInfo;

    @Override
    public Response toResponse(AuthenticationFailedException exception) {
        // on purpose change to 5xx
        return Response.status(500).entity("Custom body").build();
    }
}

Expected behavior

Response can be customized.

Actual behavior

Always returns the same response:

$ curl 'http://localhost:8080/hello' -u scott:wrong -v
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
* Server auth using Basic with user 'scott'
> GET /hello HTTP/1.1
> Host: localhost:8080
> Authorization: Basic c2NvdHQ6cmVhZGU=
> User-Agent: curl/7.68.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 401 Unauthorized
* Authentication problem. Ignoring this.
< www-authenticate: basic realm="Quarkus"
< content-length: 0
< 
* Connection #0 to host localhost left intact

How to Reproduce?

Here is the reproducible example: https://github.com/ivansenic/quarkus-25732

Output of uname -a or ver

Linux ise-Precision-5550 5.13.0-41-generic #46~20.04.1-Ubuntu SMP Wed Apr 20 13:16:21 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "17.0.3" 2022-04-19 OpenJDK Runtime Environment (build 17.0.3+7-Ubuntu-0ubuntu0.20.04.1) OpenJDK 64-Bit Server VM (build 17.0.3+7-Ubuntu-0ubuntu0.20.04.1, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.9.1.Final

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

Maven home: /home/ise/.m2/wrapper/dists/apache-maven-3.8.4-bin/52ccbt68d252mdldqsfsn03jlf/apache-maven-3.8.4 Java version: 17.0.3, vendor: Private Build, runtime: /usr/lib/jvm/java-17-openjdk-amd64 Default locale: en_US, platform encoding: UTF-8 OS name: "linux", version: "5.13.0-41-generic", arch: "amd64", family: "unix"

Additional information

No response

@ivansenic ivansenic added the kind/bug Something isn't working label May 23, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented May 23, 2022

/cc @FroMage, @geoand, @stuartwdouglas

@ivansenic
Copy link
Contributor Author

Attached example: https://github.com/ivansenic/quarkus-25732

@sberyozkin
Copy link
Member

sberyozkin commented May 23, 2022

@ivansenic Thanks, the mapper is tested here, https://github.com/quarkusio/quarkus/blob/main/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/AuthenticationCompletionExceptionMapper.java, https://github.com/quarkusio/quarkus/blob/main/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java#L696. That test, though quite complex, only passes because the test mapper adds a RedirectUri response header.
The only difference is that that test uses Resteasy Classic, so it appears that with Resteasy Reactive it does not work

@geoand
Copy link
Contributor

geoand commented May 23, 2022

I'll have a look tomorrow

@sberyozkin
Copy link
Member

Thanks @geoand, I did not mean to say it was just a RestEasy Reactive problem :-), but it might need some tweak, I can create a draft PR with a test

@geoand
Copy link
Contributor

geoand commented May 23, 2022

Completely understood 😀

@ivansenic
Copy link
Contributor Author

Hmm but I am not registering a mapper for AuthenticationCompletionException, but for AuthenticationFailedException. Could it be that I am tragetting a wrong exception class? Can test tomorrow, and if so then this js just a documentation issue.

@geoand
Copy link
Contributor

geoand commented May 24, 2022

I just tried your sample and handling the exception with the ExceptionMapper does not work for RESTEasy Classic either.

@sberyozkin I think this is best handled by you, since I am lacking a lot of the security context.
At the very least, I think we need to update the documentation to specify under which conditions using an ExceptionMapper works.

@sberyozkin
Copy link
Member

@geoand Thanks. I see that I don't have tests for mapping AuthenticationFailedException but only for AuthenticationCompletionException and I believe users have confirmed it was working for ForbiddenException.

Perhaps intercepting AuthenticationFailedException is not even possible since it is used as a trigger to create authentication mechanism specific challenges

@geoand
Copy link
Contributor

geoand commented May 24, 2022

Perhaps intercepting AuthenticationFailedException is not even possible since it is used as a trigger to create authentication mechanism specific challenges

From what I saw from the security code, that does seem to be the case.

@ivansenic
Copy link
Contributor Author

Just checked and intercepting the AuthenticationCompletionException also does not work.. I updated the reproducible example to showcase it..

@sberyozkin
Copy link
Member

@ivansenic quarkus-oidc is throwing AuthenticationCompletionException. I'll clarify in the docs

@geoand
Copy link
Contributor

geoand commented May 25, 2022

@sberyozkin is there anything else we can do to improve here?

@sberyozkin sberyozkin self-assigned this May 25, 2022
@sberyozkin
Copy link
Member

@geoand I'll be on PTO next week, overflowing into the week which follows and looking at some other open issues right now, but yeah, definitely a few things have to be clarified.

One thing that is certain is that it is impossible to intercept security exceptions with JAX-RS mappers if the proactive authentication is enabled which is a default - the security chain will start before the JAX-RS chain.
So the only way to try to handle it with JAX-RS mappers is to start a security check only when it is determined a JAX-RS method requires it, which is what disabling the proactive authentication does.
I'll review various security exceptions reported and test which ones can be intercepted and which are not in a couple of weeks.

2 other possible options:

  • custom HttpAuthenticationMechanism - it might be the only way to intercept AuthenticationFailedException since it triggers a call into HttpAuthenticationMechanism.getChallenge - and this flow must not be broken by interposing exception mappers. @ivansenic if it can be of interest then I can help with a custom mechanism, it is straightforward, one would just inject for ex, BasicAuthenticationMechanism into a custom one and delegate to it and only postprocess a getChallenge response.
  • Wire in Vert.x fault handlers into a security code handling the errors, but I'd appreciate some help from Vert.x experts here

@geoand
Copy link
Contributor

geoand commented May 25, 2022

Thanks a lot @sberyozkin!

@ivansenic
Copy link
Contributor Author

@sberyozkin In turns our that I am already implementing a custom HttpAuthenticationMechanism.. Actually it's a straight forward and standard header-based authentication.. Let me try to hook into that getChallenge thingy.. I'll get back to you..

Btw, the header based authentication could be interesting also for contributing back to Quarkus.. Here is a quick showcase on what I did: https://github.com/stargate/stargate/blob/v2.0.0/sgv2-docsapi/src/main/java/io/stargate/sgv2/docsapi/api/common/security/HeaderBasedAuthenticationMechanism.java

@ivansenic
Copy link
Contributor Author

Yup hooking into challenge works.. Just in case that somebody needs it, here is the reference on how I am setting the custom response: https://github.com/stargate/stargate/blob/304cff3ace476737ab1309a44a20b26bed8a7ad7/sgv2-docsapi/src/main/java/io/stargate/sgv2/docsapi/api/common/security/HeaderBasedAuthenticationMechanism.java#L88-L125

As far as I am concerned, the issue is resolved.

Thanks @geoand @sberyozkin

@geoand
Copy link
Contributor

geoand commented May 25, 2022

Thanks for the update @ivansenic .

I'll leave this open because at the very least, we could use a doc update.

@sberyozkin
Copy link
Member

sberyozkin commented May 25, 2022

@ivansenic Nice one, thanks for quickly verifying the custom mechanism option.
@geoand Yeah, I'll review which security exceptions can be mapped, and add a note re dealing with AuthenticationFailedException

@sberyozkin
Copy link
Member

Will look further into it asap, I haven't forgotten

@sberyozkin
Copy link
Member

sberyozkin commented Oct 12, 2022

@michalvavrik Hi Michal, did your PR fix this issue as well ? See also #26314

@michalvavrik
Copy link
Contributor

michalvavrik commented Oct 12, 2022

@sberyozkin AuthenticationFailedException here is thrown by io.quarkus.elytron.security.runtime.ElytronPasswordIdentityProvider#authenticate that is called from permissionCheckHandler and it's running before RR begin processing. Here DefaultAuthFailureHandler is used as it would be dangerous to remove it before RR begun processing (it would go to failure handler, probably down to the io.quarkus.vertx.http.runtime.QuarkusErrorHandler and not to the RR exception mappers.

It's actually very much related to the other tickets I work on, let me have a look. It would be nice to have a solution that covers all related cases, not just this reproducer (and it would fix proactive auth stuff too)

michalvavrik added a commit to michalvavrik/quarkus that referenced this issue Oct 12, 2022
michalvavrik added a commit to michalvavrik/quarkus that referenced this issue Oct 12, 2022
michalvavrik added a commit to michalvavrik/quarkus that referenced this issue Oct 13, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Oct 14, 2022
igorregis pushed a commit to igorregis/quarkus that referenced this issue Oct 16, 2022
igorregis pushed a commit to igorregis/quarkus that referenced this issue Oct 16, 2022
igorregis pushed a commit to igorregis/quarkus that referenced this issue Oct 16, 2022
igorregis pushed a commit to igorregis/quarkus that referenced this issue Oct 17, 2022
tmihalac pushed a commit to tmihalac/quarkus that referenced this issue Oct 27, 2022
kahowell added a commit to RedHatInsights/rhsm-subscriptions that referenced this issue Mar 9, 2023
https://issues.redhat.com/browse/SWATCH-849

I had to move `openapi.yaml` to `src/main/resources/META-INF` so that it would be picked up
by `OpenApiDocumentService`, so that API definitions could be easily referenced from
unit tests (e.g. see `ResourceRolesAllowedTest`). This also makes http://localhost:8000/q/openapi serve more complete
API including security schemes.

Three roles are defined:

- `service` - granted to `x-rh-swatch-psk` authenticated principals
- `support` - granted to `x-rh-identity` authenticated principals when `type` is `Associate`.
- `test` - granted to any authenticated principal when `SWATCH_TEST_APIS_ENABLED` is `true`.

Note that for each auth method, classes were added for each of:

- `AuthenticationRequest`-suffixed class to represent an attempt to authenticate.
- `Principal`-suffixed class to represent an authenticated.
- `AuthenticationMechanism`-suffixed class to create authentication requests.
- `IdentityProvider`-suffixed class to handle authentication success/failure logic.

The `SWATCH_TEST_APIS_ENABLED` value is defaulted to false in `stage` and `prod` profiles.
([SWATCH-882](https://issues.redhat.com/browse/SWATCH-882) will change it to `true` in stage).

I set up `MdcFilter` to add the principal to logging MDC, if authenticated.

I also added a new package `security` to encapsulate all things in authn/authz domain.

Note: I looked at getting more helpful auth failed responses, but it seems to not be supported due to when auth happens
in the quarkus resteasy lifecycle.

While not ideal, I think this is acceptable for internal APIs, as 401 can be interpreted as authn not provided or authn
failed, and 403 as authn succeeded, but authz failed. Though it seems we can customize if we switch to resteasy-reactive
and disable proactive authentication; see: quarkusio/quarkus#25732 and
https://quarkus.io/guides/security-built-in-authentication-support-concept#how-to-customize-authentication-exception-responses.
I opened [SWATCH-1025](https://issues.redhat.com/browse/SWATCH-1025) and
[SWATCH-1026](https://issues.redhat.com/browse/SWATCH-1026) to revisit later on.

Logging uses the principal name, and I implemented three strategies:

- For associate identities, the principal name is the email address.
- For X509 (mTLS) identities, the principal name is the subject DN (distinguished name).
- For PSK identities, the principal name is `urn:console.redhat.com:service:swatch`

Note, I set `quarkus.security.auth.enabled-in-dev-mode=false`, which makes development more convenient by not requiring
auth in dev mode. This can be toggled by setting `QUARKUS_SECURITY_AUTH_ENABLED_IN_DEV_MODE=true`.

Testing
=======

For convenience, first export the following environment variables into a shell to be used to make requests:

```shell
export PSK_HEADER=x-rh-swatch-psk:placeholder
export PSK_HEADER_BAD=x-rh-swatch-psk:wrong
export SUPPORT_HEADER=x-rh-identity:$(echo '
    {
      "identity": {
        "type": "Associate",
        "associate" : {
          "email": "test@example.com"
        }
      }
    }' | base64 -w0)
export X509_HEADER=x-rh-identity:$(echo '
    {
      "identity": {
        "type": "X509",
        "x509" : {
          "subject_dn": "CN=test.example.com"
        }
      }
    }' | base64 -w0)
```

First, test with `SWATCH_TEST_APIS_ENABLED=true` (default):

In a separate shell, start the service with debug logging turned on for com.redhat.swatch, and overriding RBAC to be on
in dev mode:

```shell
LOGGING_LEVEL_COM_REDHAT_SWATCH=DEBUG \
  QUARKUS_SECURITY_AUTH_ENABLED_IN_DEV_MODE=true \
  ./gradlew swatch-contracts:quarkusDev
```

First, try the DELETE endpoint without auth and then with incorrect psk:

```shell
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c \
  $PSK_HEADER_BAD
```

Both give 401 response, indicating auth is required, but not provided or rejected.

Next, try the DELETE endpoint with a good PSK:

```shell
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c \
  $PSK_HEADER
```

This time, you get a 204, and note a few important lines in the log:

`test` and `service` roles are granted to the PSK:

> Granting roles [test, service] to user: urn:console.redhat.com:service:swatch

Subsequent log messages indicate what user was involved (via MDC):

> {user=urn:console.redhat.com:service:swatch}

Repeat this test, but with associate and x509 identities:

```shell
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c \
  $SUPPORT_HEADER
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c \
  $X509_HEADER
```

Look at the logs and observe the roles granted, and principal name differences.

Now, repeat all of the above steps with the test role disabled (you can either set `SWATCH_TEST_APIS_ENABLED=false` and
restart the service, or you can change the config via http://localhost:8000/q/dev/io.quarkus.quarkus-vertx-http/config).
Notice that no principals can use the `DELETE` endpoint. Also notice that no principals are granted the `test` role.

You can test further role combinations as desired. An easy way to test a bunch quickly is to modify the `@RolesAllowed`
values on `ContractsTestingResource.deleteContractByUUID`, and adjusting the roles on a principal by modifying
`RolesAugmentor`.
kahowell added a commit to RedHatInsights/rhsm-subscriptions that referenced this issue Mar 9, 2023
https://issues.redhat.com/browse/SWATCH-849

I had to move `openapi.yaml` to `src/main/resources/META-INF` so that it would be picked up
by `OpenApiDocumentService`, so that API definitions could be easily referenced from
unit tests (e.g. see `ResourceRolesAllowedTest`). This also makes http://localhost:8000/q/openapi serve more complete
API including security schemes.

Three roles are defined:

- `service` - granted to `x-rh-swatch-psk` authenticated principals
- `support` - granted to `x-rh-identity` authenticated principals when `type` is `Associate`.
- `test` - granted to any authenticated principal when `SWATCH_TEST_APIS_ENABLED` is `true`.

Note that for each auth method, classes were added for each of:

- `AuthenticationRequest`-suffixed class to represent an attempt to authenticate.
- `Principal`-suffixed class to represent an authenticated.
- `AuthenticationMechanism`-suffixed class to create authentication requests.
- `IdentityProvider`-suffixed class to handle authentication success/failure logic.

The `SWATCH_TEST_APIS_ENABLED` value is defaulted to false in `stage` and `prod` profiles.
([SWATCH-882](https://issues.redhat.com/browse/SWATCH-882) will change it to `true` in stage).

I set up `MdcFilter` to add the principal to logging MDC, if authenticated.

I also added a new package `security` to encapsulate all things in authn/authz domain.

Note: I looked at getting more helpful auth failed responses, but it seems to not be supported due to when auth happens
in the quarkus resteasy lifecycle.

While not ideal, I think this is acceptable for internal APIs, as 401 can be interpreted as authn not provided or authn
failed, and 403 as authn succeeded, but authz failed. Though it seems we can customize if we switch to resteasy-reactive
and disable proactive authentication; see: quarkusio/quarkus#25732 and
https://quarkus.io/guides/security-built-in-authentication-support-concept#how-to-customize-authentication-exception-responses.
I opened [SWATCH-1025](https://issues.redhat.com/browse/SWATCH-1025) and
[SWATCH-1026](https://issues.redhat.com/browse/SWATCH-1026) to revisit later on.

Logging uses the principal name, and I implemented three strategies:

- For associate identities, the principal name is the email address.
- For X509 (mTLS) identities, the principal name is the subject DN (distinguished name).
- For PSK identities, the principal name is `urn:console.redhat.com:service:swatch`

Note, I set `quarkus.security.auth.enabled-in-dev-mode=false`, which makes development more convenient by not requiring
auth in dev mode. This can be toggled by setting `QUARKUS_SECURITY_AUTH_ENABLED_IN_DEV_MODE=true`.

Testing
=======

For convenience, first export the following environment variables into a shell to be used to make requests:

```shell
export PSK_HEADER=x-rh-swatch-psk:placeholder
export PSK_HEADER_BAD=x-rh-swatch-psk:wrong
export SUPPORT_HEADER=x-rh-identity:$(echo '
    {
      "identity": {
        "type": "Associate",
        "associate" : {
          "email": "test@example.com"
        }
      }
    }' | base64 -w0)
export X509_HEADER=x-rh-identity:$(echo '
    {
      "identity": {
        "type": "X509",
        "x509" : {
          "subject_dn": "CN=test.example.com"
        }
      }
    }' | base64 -w0)
```

First, test with `SWATCH_TEST_APIS_ENABLED=true` (default):

In a separate shell, start the service with debug logging turned on for com.redhat.swatch, and overriding RBAC to be on
in dev mode:

```shell
LOGGING_LEVEL_COM_REDHAT_SWATCH=DEBUG \
  QUARKUS_SECURITY_AUTH_ENABLED_IN_DEV_MODE=true \
  ./gradlew swatch-contracts:quarkusDev
```

First, try the DELETE endpoint without auth and then with incorrect psk:

```shell
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c \
  $PSK_HEADER_BAD
```

Both give 401 response, indicating auth is required, but not provided or rejected.

Next, try the DELETE endpoint with a good PSK:

```shell
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c \
  $PSK_HEADER
```

This time, you get a 204, and note a few important lines in the log:

`test` and `service` roles are granted to the PSK:

> Granting roles [test, service] to user: urn:console.redhat.com:service:swatch

Subsequent log messages indicate what user was involved (via MDC):

> {user=urn:console.redhat.com:service:swatch}

Repeat this test, but with associate and x509 identities:

```shell
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c \
  $SUPPORT_HEADER
http DELETE :8000/api/rhsm-subscriptions/v1/internal/contracts/8ece4096-bef6-4ad6-b0db-20c8e6b2a78c \
  $X509_HEADER
```

Look at the logs and observe the roles granted, and principal name differences.

Now, repeat all of the above steps with the test role disabled (you can either set `SWATCH_TEST_APIS_ENABLED=false` and
restart the service, or you can change the config via http://localhost:8000/q/dev/io.quarkus.quarkus-vertx-http/config).
Notice that no principals can use the `DELETE` endpoint. Also notice that no principals are granted the `test` role.

You can test further role combinations as desired. An easy way to test a bunch quickly is to modify the `@RolesAllowed`
values on `ContractsTestingResource.deleteContractByUUID`, and adjusting the roles on a principal by modifying
`RolesAugmentor`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/bug Something isn't working
Projects
None yet
4 participants