django-auth-adfs icon indicating copy to clipboard operation
django-auth-adfs copied to clipboard

#270, #357 - AdfsRefreshMiddleware & AdfsAuthCodeRefreshBackend

Open sdev95 opened this issue 4 months ago • 1 comments

Hi,

As mentioned in https://github.com/snok/django-auth-adfs/issues/270, https://github.com/snok/django-auth-adfs/issues/357, this would be a great addition to the library and the other PR ( https://github.com/snok/django-auth-adfs/pull/343 ) seems stale or hopefully he is having a great long holiday :). I will attempt to continue the effort from @Dejiah and @Daggaz in this PR.

I tried to solve your feedback on the previous PR @tim-schilling. The PR is not ready yet, but since I like quick feedback loops I decided to create it already. I have been working with Django for 6 months, so any general feedback besides this feature is greatly appreciated.

Feedback Security concerns about storing token data in the session: My current approach is to check the value of SESSION_ENGINE, and it will throw an assert message that says that it is not recommended.

Prevent the library from storing tokens in the session: I agree with you, we should not make a choice for all users of the library to store token data in the session. I saw feedback from @daggaz in the other PR and I like the idea of having a seperate backend class that has al the logic for doing the auth code flow with refreshing.

Move as much of the new changes to the middleware instead of the backend: I prefer a solution where we have a new backendclass.

There needs to be some documentation/tests: I will pick this up once the general direction is agreed upon.

Kind regards and thank you for any effort in reviewing, I will have time to act upon feedback upcoming weeks 👍

sdev95 avatar Jun 04 '25 09:06 sdev95