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

Fix for #118. Provide separate deployment to retrieve a key from a URL. #148

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

radcortez
Copy link
Contributor

No description provided.

@sberyozkin
Copy link
Contributor

@radcortez Hi Roberto thanks for this PR, sorry for a delay. I'm assuming this update will basically make sure that the test will run irrespective of how the application is deployed and its keys are initialized, correct ?

@radcortez
Copy link
Contributor Author

Yeah, the idea over here is that by using Arquillian multiple deployments, you could deploy an initial application that exposes the key and the second application that consumes the key depends on the deployment of the key app.

@sberyozkin
Copy link
Contributor

Hi @radcortez OK thanks. I've realized that some application servers (supporting single deployments like Thorntail or any other container which can't handle multiple deployments) may have problems. Perhaps we may split this test or come up with some other idea, otherwise I'll just mark it for JWT 2.0 where only UnresolvableKeyException without a tie-in to the time the keys are initialized, is checked...

@radcortez
Copy link
Contributor Author

Sure.

Another option could be to start a separate http server via HttpServer and expose the key over there. Not sure if we can plugin some sort of awaitability to wait for the server to get up and then deploy the Arquillian stuff. Might work.

I'll give it a try.

@sberyozkin
Copy link
Contributor

@radcortez It is a nice and neat idea, really is IMHO. I have a reservation now though about the issue validity. If the server fails at the startup if no remote location is available then it means it is not able to get hold of the rotated keys which is only possible by periodically refreshing a JWK set. If we have this HTTP server then what exactly we will test ? I mean it won't resolve the ambiguity about throwing or not throwing DeploymentException if no remote location is available.
Perhaps for 2.0 it could make sense to have such a test with the HTTP server emulating the case of returning a set with a non-matching kid, returning 404, etc, to support few variations around the remote key unavailability and verify that no matter what the client gets the same HTTP error, etc

@radcortez
Copy link
Contributor Author

Well, right now this was just to fix the issue for certain servers where the endpoint you use for the key is not available to query, because is part of the current deployment (and egg and chicken problem).

I agree that we need to discuss this further and figure out what should be the correct behaviour of the spec in these cases (404, wrong kid, etc).

@sberyozkin sberyozkin added this to the JWT-2.0 milestone Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants