Open smysnk opened 3 years ago
The issue presents the problem that doesn't apply to our library.
The token is checked only at init and kept there. This is obviously not great because if you remove roles or revoke people's tokens their subscriptions will still work and stream the data. The only one way to get around it is to restart server.
On the other hand there is no other reliable way to handle this properly. Passing token on subscription will be the same as headers (just different way)
Seems like a better approach would be to pass the bearer token as a parameter to the subscription query?
GraphQL queries are not suited to handle secrets like token
The issue presents the problem that doesn't apply to our library.
Yeah sorry, I am aware that the question was not specifically in the domain of keycloak-connection-graphql
. But having implemented a connector for keycloak / graphql you have specific knowledge that would allow you to answer this question better. Also if it was not entirely a crazy idea it could possibly be a feature request for this connector.
If I am understanding correctly, using connection based tokens as it currently implemented will allow new subscriptions to initiated even though the token has expired?
I am just wondering about the scenario: 1) 0:00 Token issued by keycloak to browser client (expires in 1:00) 2) 0:00 browser client initiates WS connection to a API client initiated with said Token 3) 1:01 A new subscription is requested by the browser client which needs to be validated against that token
Will keycloak-connection-graphql
throw an error that this token has expired?
Update: After some testing it appears the function hasRole returns false
after the token has expired. So it does appear that the above scenario is an issue.
If token is invalid (expiration is pretty low standard) subscription will not be initialized and rejected on query level.
Strangely enough hasRole is not using token for its logic.
I think I know what is happening your client is using the same ws connection for all queries. Authentication is done on Websocket level not query level. Meaning that your use case is right. I'm not sure about hasRole returning false for this - that is authorization level.
There are couple ways to mitigate this if you are uberly concerned about expiration times. Sadly they are on the client side. We have been discussing couple times with team and we could not find any business case for doing it differently.
If there is bug regarding token expiry affecting authorization it might be actually regression from last feature. Let's work on pinpointing it
I tested this in kind of a crude method using the following scenario:
import {
KeycloakSubscriptionHandler,
KeycloakSubscriptionContext,
} from 'keycloak-connect-graphql';
import Keycloak from 'keycloak-connect';
import session from 'express-session';
(async () => {
const memoryStore = new session.MemoryStore();
const keycloak = new Keycloak({
store: memoryStore,
}, {
realm: 'xxx',
'realm-public-key': 'xxx',
'auth-server-url': 'https://auth.keycloak.com/auth',
'ssl-required': 'external',
resource: 'nodejs-connect',
'public-client': true,
'bearer-only': true,
});
const keycloakSubscriptionHandler = new KeycloakSubscriptionHandler({ keycloak, protect: false });
const token = await keycloakSubscriptionHandler.onSubscriptionConnect({ Authorization: 'Bearer xxx' });
const context = new KeycloakSubscriptionContext(token);
setInterval(() => {
console.log(token.content.exp - Math.floor(Date.now() / 1000), context.hasRole('realm:test'));
}, 1000);
})();
Where at the exp hasRole returns false
instead of true
prior to exp. I am not sure if client code accurately replicates the functionality of this module if wired as intended. I am just starting implement WS auth for one of my projects, so this is the first I am digging into this and am still quite naive in this area.
This very may well be a silly idea, but I'm going to shoot it out there anyways. There seems to be some obvious limitations around authentication via websocket connectionParams. If you're using a bearer token they're likely to expire quite frequently and you would be required to re-connect the websocket each time you renew the token. Not only would that interrupt existing subscriptions, not very efficient and a bit clunky.
Seems like a better approach would be to pass the bearer token as a parameter to the subscription query?