Add Json Web Token (JWT) authenticator#10289
Conversation
dain
force-pushed
the
jwt-token-authentication
branch
2 times, most recently
from
f9a73a9 to
4dc6515
Compare
dain
changed the title
dain
force-pushed
the
jwt-token-authentication
branch
2 times, most recently
from
e97bc79 to
ace92da
Compare
dain
changed the title
dain
force-pushed
the
jwt-token-authentication
branch
from
ace92da to
bc75964
Compare
dain
force-pushed
the
jwt-token-authentication
branch
from
bc75964 to
a1f475a
Compare
electrum
left a comment
electrum
left a comment
There was a problem hiding this comment.
Typo "autentication" in the commit message
There was a problem hiding this comment.
This needs to catch JwtException (and maybe others) and call needAuthentication() with the message. A RuntimeException thrown from authenticate() will cause the server to return a 500, so that should only happen if there is an internal error, not due to an expired token or similar.
There was a problem hiding this comment.
Those errors should only happen if the token is not trusted or corrupted. In other authenticators, they throw runtime when this happens. I think we should have another mechanism to communicate these kinds of errors.
There was a problem hiding this comment.
If you don't set is, all issuers are allowed.
There was a problem hiding this comment.
Static import the assertion methods
There was a problem hiding this comment.
Use the constant AUTHORIZATION like the method above
There was a problem hiding this comment.
Should we do validation to make sure the token is only printable ASCII? I'm wondering what happens if you put in a newline, etc.
There was a problem hiding this comment.
What happens for username and password?
There was a problem hiding this comment.
The password is base64 encoded. I suppose we should have validation for username and other headers.
There was a problem hiding this comment.
Is this needed? We do a force refresh in TestingPrestoServer. I don't know why this is in TestPrestoDriver
There was a problem hiding this comment.
If we do need this, import the one from TestPrestoDriver
There was a problem hiding this comment.
Import the method from TestPrestoDriver
|
Kerberos and password both have normal authentication failures (different from errors). Corrupt or expired should be a normal auth failure. See how we handle missing colon or invalid base64 in PasswordAuthenticator.
|
dain
force-pushed
the
jwt-token-authentication
branch
from
a1f475a to
a9e3191
Compare
pom.xml
Outdated
There was a problem hiding this comment.
Might be better to add an exception for maven-dependency-versions-check-plugin below, as these are actually required but the version is wrong
dain
force-pushed
the
jwt-token-authentication
branch
2 times, most recently
from
137c35d to
891c425
Compare
dain
force-pushed
the
jwt-token-authentication
branch
from
891c425 to
c6d3d6c
Compare
3 participants
No description provided.