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

Impossible test in JWT 1.1 TCK #103

Closed
arjantijms opened this issue Aug 21, 2018 · 5 comments
Closed

Impossible test in JWT 1.1 TCK #103

arjantijms opened this issue Aug 21, 2018 · 5 comments
Assignees
Labels
TCK challenge An issue that challenges a TCK test in some way
Milestone

Comments

@arjantijms
Copy link

The JWT TCK contains the following test:

https://github.com/eclipse/microprofile-jwt-auth/blob/master/tck/src/test/java/org/eclipse/microprofile/jwt/tck/container/jaxrs/PrincipalInjectionEndpoint.java#L58

This is however impossible to pass in general. Principal is a proxy, and in CDI the only guarantee you have is that the proxy implements the type of the injection point.

In other words, you can't do instanceof tests on the proxy for either types, and thus this test will never pass.

I propose removing this test.

@Emily-Jiang
Copy link
Member

Proxy is a subclass. Why cannot the instanceof be done on the proxy? By the way, you need to provide an alternative Principal bean other than the in-built Principal.

@arjantijms
Copy link
Author

Why cannot the instanceof be done on the proxy?

Because in CDI there's no guarantee on what the actual class of the proxy is. The only guarantee is that it is the type that the injection target has. This is a well known "problem" (some would say it's by design) in CDI.

See https://issues.jboss.org/browse/CDI-10 and more directly https://issues.jboss.org/browse/CDI-597

By the way, you need to provide an alternative Principal bean other than the in-built Principal.

I'm not sure that is needed. Where would it say that?

If you set the principal in the right way using Java EE's SPIs, then everything else will see that principal (i.e. HttpServletRequest.getUserPrincipal, EJBContext.getCallerPrincipal, SecurityContext.getCallerPrincipal, and of course the standard @Inject Principal).

Providing an alternative Principal bean would be a very local override (akin to providing a wrapped HttpServletRequest etc), and should not be needed. In fact, it's actually mostly unwanted as you run the risk to be out of sync with the other APIs to obtain that principal.

@arjantijms
Copy link
Author

@Emily-Jiang just to be sure the @injected Principal by CDI, is the JsonWebToken. If you would add a bunch of methods to it that getName() would call, then these would be called correctly on the injected principal. You just can't cast it.

You can cast the Principal returned from an injected SecurityContect to JsonWebToken.

@starksm64 starksm64 self-assigned this Aug 22, 2018
@starksm64 starksm64 added the TCK challenge An issue that challenges a TCK test in some way label Aug 22, 2018
@Emily-Jiang
Copy link
Member

@arjantijms yes, what you said is the mandated by the spec. Principal you get will be JsonWebToken.

starksm64 added a commit to MicroProfileJWT/microprofile-jwt-auth that referenced this issue Aug 26, 2018
…nWebToken

Validate SecurityContext#getUserPrincipal as JsonWebToken
@starksm64
Copy link
Contributor

The test has been updated to validate that the Principal name matches rather than being castable, and that SecurityContext#getUserPrincipal does cast to JsonWebToken.

starksm64 added a commit to MicroProfileJWT/microprofile-jwt-auth that referenced this issue Aug 27, 2018
…cation URL base.

Move the test version enum to the tck api jar and update TCK readme.
starksm64 added a commit to MicroProfileJWT/microprofile-jwt-auth that referenced this issue Aug 27, 2018
starksm64 added a commit to MicroProfileJWT/microprofile-jwt-auth that referenced this issue Aug 27, 2018
…ce to indicate which version of MP-JWT the test is targeting, and document the expected behavior in TCK readme.
starksm64 added a commit to MicroProfileJWT/microprofile-jwt-auth that referenced this issue Aug 27, 2018
starksm64 added a commit that referenced this issue Aug 27, 2018
@starksm64 starksm64 added this to the 1.1.1 milestone Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TCK challenge An issue that challenges a TCK test in some way
Projects
None yet
Development

No branches or pull requests

3 participants