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

Support @TestSecurity in combination with JWT-Security #11695

Closed
janmaterne opened this issue Aug 28, 2020 · 8 comments · Fixed by #16362
Closed

Support @TestSecurity in combination with JWT-Security #11695

janmaterne opened this issue Aug 28, 2020 · 8 comments · Fixed by #16362

Comments

@janmaterne
Copy link

Description
If your application uses JWT for security, you could disable the security during tests with @TestSecurity(authorizationEnabled = false).
But if your application requires data (like username or roles) from the JWT, the test will crash with a HTTP-500.

@Inject
@Claim(standard = Claims.preferred_username)
String currentUserName;

@Inject
@Claim(standard = Claims.email)
String email;
	
@Inject
@Claim(value = "realm_access")
String rolesFromRedhatSSO725;

Implementation ideas
Generate a JsonWebToken with the data from the @TestSecurity annotation and publish it to CDI context for later injection.

The io.quarkus.test.security.QuarkusSecurityTestExtension#beforeEach does this for the SecurityIdentity, but adding JWT here would add another dependency which is not required everywhere. Maybe directly a json?

Maybe a hook could be implemented where the test author could add some custom values for the JWT.

@Test
@TestSecurity(name = "username", roles = {"user", "admin"} )
@TestJwtData(key="field1", value="value1")
@TestJwtData(key="field2", value="value2")
void customJwtFields() {
}
@quarkusbot
Copy link

/cc @radcortez

@sberyozkin
Copy link
Member

@janmaterne thanks, I wonder if we can have TestAttribute (similar to your TestJwtData) which by default will be added to SecurityIdentity attributes, but also have a TestSecurity type property:

@Test
@TestSecurity(name = "username", roles = {"user", "admin"}, type="jwt" )
@TestAttributes(
  @TestAttribute(key="name", value="value"),
  @TestAttribute(key="name", value="value")
)

and this type will activate a test JWT handler instead of the default TestSecurity one.
What do you think ?
CC @stuartwdouglas

@janmaterne
Copy link
Author

Sounds good. The value of the type-attribute will specify which handler/strategy to use.

Maybe that could default to the value which is used by the application. Could be done by checking for the existence of marker classes on the classpath (during build time):

io.quarkus.smallrye.jwt.runtime.auth.MpJwtValidator --> jwt
...
default '?'

First thought of the name @TestAttribute was, that it is more related to the test than the security. So a @TestSecurityAttribute would be more clearer (but also longer ...).

An alternative would be additional values for the security annotation

@TestSecurity(name="username", roles={"user", "admin"}, type="jwt",
    attributes={
        @TestAttribute(key="name", value="value"),
        @TestAttribute(key="name", value="value")
    }
)

But I would prefer your direct use as it is more easier to write than this nested annotations.

@sberyozkin
Copy link
Member

sberyozkin commented Aug 28, 2020

@janmaterne thanks, I actually like your last example, as I guess it all has to be better off grouped together, such extra attributes will not always be used; Stuart will have a better appreciation of it though.
The default mechanism idea is also good, perhaps it can be realized via an extra PR, there could be more than one mechanism loaded so having a type or something similar will likely offer better guarantees.
You've raised a good issue because we can also have, with OIDC, @IdToken @Inject JsonWebToken jwt; etc.
Perhaps we will need to have

test-framework/test-security
test-framework/test-security-jwt // provides CDI handler which will convert all of @TestSecurity data into JsonWebToken
principal 
//etc for other auth mechanisms

@janmaterne
Copy link
Author

janmaterne commented Aug 29, 2020

When using a nested structure maybe another name is better:

@TestSecurity(name="username", roles={"user", "admin"}, type="jwt",
    attributes={
        @SecurityAttribute(key="name", value="value"),
        @SecurityAttribute(key="name", value="value")
    }
)
  • name: specifies the username for the test request (required)
  • roles: specifies the roles for this user (not required, defaults to no rule)
  • type: specifies the type of the generated injectable types
    • jwt: JsonWebToken
    • oidc: ...
    • ...: ...
      not required, it defaults to the used security infrastructure the application
      depends on.
      TODO: is support of multiple types required?
  • attributes: List of additional data the generated CDI bean should contain.
    not required, defaults to empty list

@geoandri
Copy link
Contributor

Hi @sberyozkin ,
I am a bit lost with this.
Should I replicate something similar to the way the SecurityIdentity is built and added to the CDI ?

CDI.current().select(TestIdentityAssociation.class).get().setTestIdentity(user);

I am a bit confused about the role of those *Association classes used. Are there any related docs to help me clear things out?

@sberyozkin
Copy link
Member

sberyozkin commented Dec 4, 2020

@geoandri Hi, I've missed it, sorry. I think this issue is about making sure that that setTestIdentity methid accepts a Secutiyidentity instance with its Principal being a JsonWebToken instance created as required by TestSecurity annotation

@geoandri
Copy link
Contributor

geoandri commented Dec 7, 2020

Hi @sberyozkin,

thanks for your help. I have submitted a PR.

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