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

Using authOptional in :api pipelines #97

Closed
LuchoTurtle opened this issue Jan 11, 2023 · 6 comments · Fixed by #102
Closed

Using authOptional in :api pipelines #97

LuchoTurtle opened this issue Jan 11, 2023 · 6 comments · Fixed by #102

Comments

@LuchoTurtle
Copy link
Member

I'm trying to use this plug in an :api pipeline.

Having read the documentation, doing

pipeline :authoptional, do: plug(AuthPlugOptional, %{})

should be enough to get up and going.
Adding this pipeline to the scope should be fairly simple, like so:

  scope "/api", AppWeb do
    pipe_through [:api, :authOptional]

    resources "/items", ItemController, only: [:create, :update]
  end

However, I keep stumbling upon this error when calling the app anonymously.

session not fetched, call fetch_session/2

This error doesn't occur when I add a Bearer Token, for example.

This error occurs in auth_plug and I've located where. The error is in get_jwt, when calling get_session..

It's weird, this error shouldn't be happening according to the Conn.Plug's source code, since the :jwt atom is being passed.

I'm aware this could easily be surpassed by just creating the authOptional pipeline with a plug:fetch_session, like so:

  pipeline :authOptional do
    plug :fetch_session
    plug(AuthPlugOptional)
  end

However, I feel like auth_plug should gracefully handle these scenarios.

Should a guard be added in get_jwt? Or is it the user's responsibility to fetch_session in their :authOptional pipelines?

@nelsonic
Copy link
Member

@LuchoTurtle great question, thank you for asking/opening it. 👌
I’ve wondered the same in the past and opted for allowing each consuming app to implement the :fetch_session but if there was no negative side effect of doing it in auth_plug is be happy to include it in the library. 💭

@LuchoTurtle
Copy link
Member Author

I've pondered about this and tried to find a way to handle this but perhaps it's best for the user to do the fetching of the session themselves.

Auth_plug shouldn't fetch the session just to verify if it exists. This will cause fetching the session two times when using a browser pipeline, which is not really bueno.

@nelsonic
Copy link
Member

Exactly. Thanks for confirming. 👌

@nelsonic
Copy link
Member

@LuchoTurtle do you want to add a brief section on how to use auth_plug in an API pipeline to the README.md? 🙏

@LuchoTurtle
Copy link
Member Author

Just created #102 to briefly explain how to use this package with APIs. Nothing biggie, just a clarification.

@github-project-automation github-project-automation bot moved this from 🔖 Ready for Development to ✅ Done in dwyl app kanban Jan 19, 2023
@nelsonic
Copy link
Member

Thanks very much @LuchoTurtle 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants