search

tanguilp / plugoid

OpenID Connect Plug for Elixir's Phoenix web framework
https://hexdocs.pm/plugoid/
Apache License 2.0
16 stars 1 forks source link

Config to save requested claims #12

Closed brianmay closed 2 years ago

brianmay commented 2 years ago

Fixes #7

brianmay commented 2 years ago

Note this will break existing sessions. Not sure how much that matters...

brianmay commented 2 years ago

dialyzer passes. Which is somewhat amazing.

I can't test act stuff, but hopefully I didn't break it (except for existing sessions). Everything else looks good to me.

tanguilp commented 2 years ago

The problem with this approach is that cookies have size limit, and authentication cookies can hold authentication info of several sessions. This could be a problem when using the COOKIE backend (data stored in the cookie and not in a backend).

This would be nice if we could configure which claims are saved, and which ones are not.

tanguilp commented 2 years ago

There's also the alternative which consist in giving the opportunity to the user to to whatever he wants with the claims (and tokens) and the Plug.Conn.t, for example saving them in its own session. See #7

brianmay commented 2 years ago

This PR does allow specifying which claims you do/don't want to keep. Not sure I would ever need any more then this?

But yes, if it supported a function as well as a list, I guess it could be more generic.

brianmay commented 2 years ago

Feel free to take this PR and modify more in line of what you want.

tanguilp commented 2 years ago

I've taken the second approach: in the conn_callback branch the token callback takes the conn as a parameter and returns it. It should be sufficient to set whatever you want in the session.

https://github.com/tanguilp/plugoid/tree/conn_callback

Feel free to test it and give me feedback.

brianmay commented 2 years ago

Thanks. Will test ASAP.

I imagine this means the callback can put data in the default phoenix session, which would solve one of the problems we were predicting with the live view not having access to the required session details.

Wondering if the special case code for subject is still required though. We might want to include it in the default session also, so that live views have access. Which would mean we get two copies.

brianmay commented 2 years ago

I am going to put this URL here so I don't have to keep finding it. Seems the appropriate place anyway:

https://github.com/tanguilp/plugoid/compare/conn_callback

brianmay commented 2 years ago

So far, your branch seems to work fine.

brianmay commented 2 years ago

If you want to merge your branch, we can then close this PR.

brianmay commented 2 years ago

Will close and summarise in #7.