vert-x3 / vertx-examples

Vert.x examples
Apache License 2.0
3.55k stars 2.09k forks source link

An example for manually adding user object to session store #393

Closed jsoneaday closed 1 year ago

jsoneaday commented 4 years ago

Describe the feature

Get an example that shows how to add an authenticated user object into session store after successful login

Use cases

This will allow subsequent routing requests to check for the user object, instead of erroneously asking for authentication again.

Contribution

This should be done by team that supports vertx-web, since this is where the session is created

tsegismont commented 4 years ago

@pmlopes this issue is related to a question on gitter about performing login with a GraphQL mutation. It seems to me that the requirement is not much different than what we do in https://github.com/vert-x3/vertx-web/blob/master/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/FormLoginHandlerImpl.java, am I right?

The user must set-up an AuthProvider and then have code similar to FormLoginHandler in the mutation data fetcher.

pmlopes commented 4 years ago

@tsegismont, @jsoneaday Yes, to some extent yes, but if this is a common practice (auth on a data fetcher), we could have a default implementation on graphql, wdyt?

And of course a default implementation to perform authz?

pmlopes commented 4 years ago

Bear with me as I'm not a graphql expert :) if we create a resolver, that we have a custom VertxDataFetcher that is created with a AuthenticationProvider. This data fetcher could verify the user and set it to the context, and on success delegate to a second one that would really do what the end user wanted.

In pseudo-code, something like:

AuthenticationDataFetcher.create(JWTAuthProvider.create())
  .authenticate((env, fut) -> fut.complete(getAllLinks(env)))

So it would perform the needed authn task and set the user in the routing context.

Then maybe a second one

AuthorizationDataFetcher.create(AuthorizationHandler.create("ADMIN_ROLE"))
  .authorize((env, fut) -> fut.complete(getAllLinks(env)))
pmlopes commented 4 years ago

Maybe we could even drop the chaining to the user getAllLinks() as the API is based on Promise/Future and we can chain then easily right?

jsoneaday commented 4 years ago

Hello

What is getAllLinks what does it do? Also once authenticated will this user object be available to any other graphql datafetcher?

How will expiration of the user object happen? The user should expire eventually.


From: Paulo Lopes notifications@github.com Sent: Thursday, June 4, 2020 8:18 AM To: vert-x3/vertx-examples vertx-examples@noreply.github.com Cc: jsoneaday dharric@live.com; Mention mention@noreply.github.com Subject: Re: [vert-x3/vertx-examples] An example for manually adding user object to session store (#393)

Bear with me as I'm not a graphql expert :) if we create a resolver, that we have a custom VertxDataFetcher that is created with a AuthenticationProvider. This data fetcher could verify the user and set it to the context, and on success delegate to a second one that would really do what the end user wanted.

In pseudo-code, something like:

AuthenticationDataFetcher.create(JWTAuthProvider.create()) .authenticate((env, fut) -> fut.complete(getAllLinks(env)))

So it would perform the needed authn task and set the user in the routing context.

Then maybe a second one

AuthorizationDataFetcher.create(AuthorizationHandler.create("ADMIN_ROLE")) .authorize((env, fut) -> fut.complete(getAllLinks(env)))

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fvert-x3%2Fvertx-examples%2Fissues%2F393%23issuecomment-638812053&data=02%7C01%7C%7Cd2849f5a269b46453e4808d808817456%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637268699372013757&sdata=m9KiYWaYfXFw1bBDCiNCYZIxMcO8qhZxO5AcBRkFBcQ%3D&reserved=0, or unsubscribehttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAALIC54QQXLDDHMYLQFA3CTRU6GLBANCNFSM4NR4QUPQ&data=02%7C01%7C%7Cd2849f5a269b46453e4808d808817456%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637268699372023750&sdata=wINII%2B6j6Gfq63xyUUGrselu%2FUN1TVxfDlRyq%2BJpgLI%3D&reserved=0.

pmlopes commented 4 years ago

getAllLinks() is a fictitious function that is provided by the user, that represents the logic to real fetch data from somewhere

pmlopes commented 4 years ago

The user interface has a isExpired() method, that will perform some logic if the provider included a expiration time. so checks can be added. Usually these are implemented in the AuthenticationProviders but nothing blocks from being checked again in other places

jsoneaday commented 4 years ago

Thanks @pmlopes . According to Gerard Klijs in this post https://spectrum.chat/graphql-java/general/i-have-a-login-mutation-if-login-is-successful-i-would-like-to-add-the-user-object-to-the-context-or-environment~75b37761-e50c-4a1e-8890-618dedd96073 each graphql call gives a new env and context. Will the user object exist on subsequent calls by other datafetchers?

I am using io.vertx.ext.auth.User but see no isExpired method.

pmlopes commented 4 years ago

@jsoneaday expired() is a newly added feature on milestone5: https://github.com/vert-x3/vertx-auth/blob/master/vertx-auth-common/src/main/java/io/vertx/ext/auth/User.java#L65

Regarding the post, as I wrote, I don't know much about the internals of graphql, I assumed that the environment would be shared for all data fetchers per request

jsoneaday commented 4 years ago

Thanks @pmlopes

Just an fyi when I use m5 I see that setAuthProvider signature for AuthProvider says AuthProvider is deprecated.

override fun setAuthProvider(authProvider: io.vertx.ext.auth.AuthProvider?) { this.authProvider = authProvider }

pmlopes commented 4 years ago

Yes, we decoupled the provider from the user, this allows having a single implementation (not one per provider) plus you can also mix, say for example, authenticate users using a token and authorize using a database.

Before all has locked to the provider. And was a long standing request to be fix from the community.

Sadly as we want to help users to quickly migrate from 3 to 4 lots of stuff is deprecated to assist with it instead of a large break that would force a fix and rebuild for everyone.

Later we will follow the usual guidelines of removing deprecated stuff older that 3 non bugfix releases and will get cleaner apis

jsoneaday commented 4 years ago

@tsegismont I saw your comment in gitter about using the session so I tried it. I think I'm pretty close but the user is not found in the session. Let me show you my code please see if there's anything obviously wrong with it. router.route().handler(SessionHandler.create(LocalSessionStore.create(vertx))) router.route("/login").handler{ ctx -> println("start login user") val pgAuthProvider = PgAuthProvider(vertx, config) val userName = "test1" // ctx.request().getParam("userName") val password = "Test123%$" // ctx.request().getParam("password") val credentials = json { obj( "userName" to userName, "password" to password ) } pgAuthProvider.authenticate(credentials) { userResult -> if(userResult != null) { val user = userResult.result() ctx.setUser(user) ctx.session().put("user", user) println("Check user added to session, ${ctx.session().get<User>("user")}") } else { println("User not found") ctx.fail(401) } } }

First I create the local session store and then I create a new route called login that manually runs authenticate. Inside of authenticate's handler I set the user into the context session and I even test print it to make sure it is in there (which it is). Then I do this in my graphql datafetcher.

` val getLoggedInUserFetcher = object : VertxDataFetcher<User?> { override fun get(environment: DataFetchingEnvironment?): CompletionStage<User?>? { val future = CompletableFuture<User?>()

  val context = environment?.getContext<RoutingContext>()
  println("Datafetcher context $context")
  println("Datafetcher session ${context?.session()}")
  println("Datafetcher session user ${context?.session()?.get<io.vertx.ext.auth.User>("user")}")
  future.complete(context?.session()?.get<Any>("user") as User)

  return future
}

} `

Both the context and the session come back as non-null objects. However .get("user") comes back null. Why is this?

jsoneaday commented 4 years ago

If we can get this to work. I'll be happy to create a small write-up you can add to your documentation. I'm sure other people will want to use this method.

jsoneaday commented 4 years ago

Interesting when I do this, context?.session()?.data()?.size I get 0. Seems like nothing is making its way into the VertxDataFetcher?

tsegismont commented 4 years ago

@tsegismont, @jsoneaday Yes, to some extent yes, but if this is a common practice (auth on a data fetcher), we could have a default implementation on graphql, wdyt?

@pmlopes I'm not sure we need GraphQL specific code. I think it would be better to have the logic that is present in FormLoginHandler extracted in a helper.

And of course a default implementation to perform authz?

@pmlopes Vert.x Auth has programmatic authorization checks so I think we don't need anything else here.

If we can get this to work. I'll be happy to create a small write-up you can add to your documentation. I'm sure other people will want to use this method.

@jsoneaday I'll work on an example and keep you informed

tsegismont commented 4 years ago

@pmlopes @jsoneaday here's a demo of login/logout with graphql mutations and web sessions.

Auth is a provided by HtpasswdAuth, initialized in the verticle start method:

auth = HtpasswdAuth.create(vertx, new HtpasswdAuthOptions().setHtpasswdFile("users"));

There are two users: john (password john) and sarah (password sarah). Of course this can be adapted to whichever supported auth provider is needed.

In particular, look at the login data fetcher:

  private CompletionStage<Boolean> login(DataFetchingEnvironment env) {
    CompletableFuture<Boolean> future = new CompletableFuture<>();

    String username = Objects.requireNonNull(env.getArgument("user"), "user is null");
    String password = Objects.requireNonNull(env.getArgument("password"), "password is null");

    RoutingContext rc = env.getContext();
    Session session = rc.session();
    JsonObject authInfo = new JsonObject()
      .put("username", username)
      .put("password", password);
    auth.authenticate(authInfo, ar -> {
      if (ar.succeeded()) {
        User user = ar.result();
        rc.setUser(user);
        if (session != null) {
          session.regenerateId();
        }
        future.complete(true);
      } else {
        ar.cause().printStackTrace();
        future.complete(false);
      }
    });
    return future;
  }

and logout:

  private boolean logout(DataFetchingEnvironment env) {
    RoutingContext rc = env.getContext();
    rc.clearUser();
    rc.session().destroy();
    return true;
  }

@pmlopes I believe there is no need for a special auth data fetcher that wraps another one. We could put this demo in vertx-examples though.

@jsoneaday please try the demo and try to adapt it in your application. Feel free to give your feedback here.

jsoneaday commented 4 years ago

In my example I use a standard route to login and then try and use that session in my graphql datafetcher, which has an empty session and user object for some reason. In your example you move the login code into the graphql datafetcher. I still don't understand why my code does not work? Is the session object going to graphql somehow different than the one being used in standard routes?

I suppose regenerateId is creating a new session id?

I'll try your method as for the most part that's the code I had originally, but with my own custom postgres auth provider. Thanks!

tsegismont commented 4 years ago

In my example I use a standard route to login and then try and use that session in my graphql datafetcher, which has an empty session and user object for some reason. I still don't understand why my code does not work? Is the session object going to graphql somehow different than the one being used in standard routes?

Can you put your whole sample on GH as well so I can take a look?

In your example you move the login code into the graphql datafetcher

Yes my understanding was that you wanted to login with a mutation instead of relying on transport only mechanism.

I suppose regenerateId is creating a new session id?

Yes. It is recommended by OWASP

I'll try your method as for the most part that's the code I had originally, but with my own custom postgres auth provider. Thanks!

You're welcome. Looking forward to your feedback

jsoneaday commented 4 years ago

Just to clarify I was using a standard route after I saw your comment about sessions on gitter. Yes originally I was wanting to do it from within graphql. So thank you for your sample. I will try it.

jsoneaday commented 4 years ago

Hello It did not work for me. I've given both of you guys access to my project dzhaven-server. Please note foloowing.

  1. I have two calls that attempt to populate the session/setUser: one is a standard route called login inside file HttpVerticle and the other is a dataFetcher called loginDataFetcher inside file DataFetcher.
  2. Both calls above seem to work and add the user object into either session or setUser. However when I use get getLoggedInUserFetcher I do not see the user object at all.
  3. If you look at this image below it shows that the call to graphql does not seem to end for some reason (as indicated by button allowing stop). Could this be the issue? Am I not ending or returning the graphql dataFetcher properly? I am calling return future for each call and the return type is always CompletionStage so not sure just guessing. image
jsoneaday commented 4 years ago

Any ideas on what might be wrong?

jsoneaday commented 4 years ago

Seems to me like the context is regenerated per call like I said before.

tsegismont commented 4 years ago

I can't see anything obviously wrong in the code. I noticed though that you don't use the Vert.x Web GraphiQL handler but rather an Apollo plugin for your browser. Perhaps this prevents the session cookie to be handled correctly. Can you inspect the HTTP headers in request and reponse?

jsoneaday commented 4 years ago

This is the getLoggedInUser call image

This is the login call image

jsoneaday commented 4 years ago

Is there something specifically I should look for? Can I match the cookie to session id? The two cookies do have distinct session id.

tsegismont commented 4 years ago

By default the cookie is named "vertx-web.session"

https://github.com/vert-x3/vertx-web/blob/6cbd41549d1a208b4e5c31d9a7d45d3fa95d8c30/vertx-web/src/main/java/io/vertx/ext/web/handler/SessionHandler.java#L51

Le lun. 8 juin 2020 à 17:39, jsoneaday notifications@github.com a écrit :

Is there something specifically I should look for? Can I match the cookie to session id?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vert-x3/vertx-examples/issues/393#issuecomment-640707882, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALOLNXFH6MJDLZQZ7KFQB3RVUA2XANCNFSM4NR4QUPQ .

jsoneaday commented 4 years ago

There's something else going on here. I've just tried by using a react client, which def should work. And the login call does work, but when the getLoggedInUser is called there is nothing in either the user object or the session. I've updated the code if you want to take a look. I think the context is new per call.

jsoneaday commented 4 years ago

Let me know what else I can do to help.

tsegismont commented 4 years ago

@jsoneaday from the screenshots it seems the problem is the session id cookie is not sent back after login. The reason is that the backend is a different domain than the frontend.

This could help you evaluate solutions https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials

jsoneaday commented 4 years ago

According to that link I have to add Access-Control-Allow-Credentials, true to the response. How can I do this with a graphql call?

jsoneaday commented 4 years ago

I just tried it by adding the header and also onto the response, but that did not work. I've committed updated code

tsegismont commented 4 years ago

Not only Access-Control-Allow-Credentials must be set, but the fetch or XMLHttpRequest object must be configured to include credentials.

jsoneaday commented 4 years ago

Still no go. I added the Access-Control-Allow-Credentials true to my response after login. Then I also added the credentials "include" to my client header. GetLoggedInUser still does not have the user object even though login uses setUser to set it. HttpVerticle has a function setupSession, is this properly setting up the session?

image

jsoneaday commented 4 years ago

I've done more research on jwt and it appears that without using sessions jwt is not secure (localstorage and cookies). So I would like to use sessions if at all possible.

jsoneaday commented 4 years ago

The session cookie is supposed to automatically get generated correct? It is not being generated at all. Is there something else I can do to force it to create a cookie?

jsoneaday commented 4 years ago

I got the cookie to generate but only when I do a fetch call, it does not generate with a graphql call. I'll keep trying. But even after the cookie is generated with fetch calls the session does not survive between calls.

jsoneaday commented 4 years ago

Ok I got the session to survive between standard route calls, but not between graphql calls. I think the spectrum thread I was on is correct and the routingcontext is recreated on every call unless we use the ExecutionInput. But none of your samples show how to use that https://spectrum.chat/graphql-java/general/i-have-a-login-mutation-if-login-is-successful-i-would-like-to-add-the-user-object-to-the-context-or-environment~75b37761-e50c-4a1e-8890-618dedd96073

tsegismont commented 4 years ago

The Apollo Client doc tells you how to include cookies for auth: https://www.apollographql.com/docs/react/networking/authentication/#cookie

jsoneaday commented 4 years ago

Yes and I did add the credentials to the apolloclient and the response of access-control-allow-credentials to the graphql call. It does not save across calls. Again it appears to recreate the context per call.

jsoneaday commented 4 years ago

Ok so I think I'm close. Like I said I was able to pass the client side header credentials include, but once I do that I get cors errors like this below (I have cors enabled to my client url). Now the reason this is strange is because in my graphql calls I am sending response header of "Access-Control-Allow-Credentials" true. So this seems to have no effect. Is there some other way of sending headers from datafetchers? I've included my client ApolloClient setup too.

Access to fetch at 'http://localhost:8888/graphql' from origin 'http://localhost:3000' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: The value of the 'Access-Control-Allow-Credentials' header in the response is '' which must be 'true' when the request's credentials mode is 'include'.

const apolloClient = new ApolloClient({ request: (operation) => { operation.setContext({ headers: { credentials: "include", }, }); }, credentials: "include", uri: "http://localhost:8888/graphql", cache: new InMemoryCache(), });

jsoneaday commented 4 years ago

Just to clarify the same setup works from standard vertx routes using fetch on the client and response headers from server. For some reason it appears that the response header from graphql calls is not getting out.

Access to fetch at 'http://localhost:8888/graphql' from origin 'http://localhost:3000' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: The value of the 'Access-Control-Allow-Credentials' header in the response is '' which must be 'true' when the request's credentials mode is 'include'.

jsoneaday commented 4 years ago

Ok I got it! There is a flag on cors handler called allowCredentials. This whole time I've been trying to respond with a header, but for whatever reason that is ignored. So you have to set this flag on the cors handler in vertx-web.

I'll be doing a video on this but if you want I can create a small write up so you can add it to your documentation.

jsoneaday commented 4 years ago

I created a. video about how I got this sessions, cors, graphql, vertx, and react all working. https://youtu.be/nsuCuKkBoQE