snok / django-auth-adfs

A Django authentication backend for Microsoft ADFS and AzureAD
http://django-auth-adfs.readthedocs.io/
BSD 2-Clause "Simplified" License
270 stars 98 forks source link

Why not pass authorize code/access_token when oauth2/callback redirect to LOGIN_REDIRECT_URL? #267

Open aoiheaven opened 1 year ago

aoiheaven commented 1 year ago

Can we pass authorization code (or even access token) to redirect, so that we could handle access token to query other data(photo/...) with graph api? https://github.com/snok/django-auth-adfs/blob/896d65bbe5c0ba2267101521189ac6cf881e08b5/django_auth_adfs/views.py#L29 https://github.com/snok/django-auth-adfs/blob/896d65bbe5c0ba2267101521189ac6cf881e08b5/django_auth_adfs/views.py#L60

or maybe there are some other ways to achieve my proposal?

Fund with Polar

JonasKs commented 1 year ago

Hmm, one way is to store the access token on the user object. Storing these secrets in a database always involves some security concerns, though.

@tim-schilling , any thoughts? 🤔

tim-schilling commented 1 year ago

That token (request.GET.get('code')) is a one-time use token. To get the long lasting token from AdfsAuthCodeBackend.authenticate / AdfsBaseBackend.exchange_auth_code.

I would imagine the token you actually want is the one that the exchange was processed for. Something custom can be done by overriding this function. Or you can hook into the post_authenticate signal which is passed the adfs_response dictionary which has the access token via adfs_response["access_token"].

Does that make sense @aoiheaven?

JonasKs commented 1 year ago

I don't believe we send the access token as a signal? I guess we could, though.

tim-schilling commented 1 year ago

@JonasKs we do already. It's in adfs_response dict in the signal. The receiver needs to pull it out of the dict.

JonasKs commented 1 year ago

Ohhh, you're right! Sorry.

tim-schilling commented 1 year ago

No worries, I didn't explain it very well.

aoiheaven commented 1 year ago

That token (request.GET.get('code')) is a one-time use token. To get the long lasting token from AdfsAuthCodeBackend.authenticate / AdfsBaseBackend.exchange_auth_code.

I would imagine the token you actually want is the one that the exchange was processed for. Something custom can be done by overriding this function. Or you can hook into the post_authenticate signal which is passed the adfs_response dictionary which has the access token via adfs_response["access_token"].

Does that make sense @aoiheaven?

Sol 2. signal callback is exactly what I need! Below was my implementation:

# In my extended user model (by OneToOneField)

from django.db import models
from django.contrib.auth.models import User
from django.db.models.signals import post_save
from django.dispatch import receiver
from django_auth_adfs.signals import post_authenticate

# Create your models here.
class XXXXUser(models.Model):
    user = models.OneToOneField(User, on_delete=models.CASCADE, related_name="profile")
    ...
    access_token = models.TextField(default="", null=True)

    class Meta:
        managed = True
        ...

@receiver(post_save, sender=User)
def create_user_profile(sender, instance, created, **kwargs):
    if created:
        XXXXUser.objects.create(user=instance)

@receiver(post_save, sender=User)
def save_user_profile(sender, instance, **kwargs):
    instance.profile.save()

@receiver(post_authenticate)
def callback_4rom_post_authenticate(sender, user, claims, adfs_response, **kwargs):
    current_user = XXXXUser.objects.get(user=user)
    current_user.access_token = adfs_response["access_token"]
    current_user.save()

image

Only one thing I am afraid about that can this signal could be refreshed and update adfs_response["access_token"] you know the access_token would expire about 1hour and need to refresh or query again, can this package also support to update access_token? @tim-schilling @JonasKs

tim-schilling commented 1 year ago

the access_token would expire about 1hour and need to refresh or query again, can this package also support to update access_token?

I'm not sure I understand what you want exactly. What would this entail?

Have you looked at these two functions to see how the package interacts with [AD here](the access_token would expire about 1hour and need to refresh or query again, can this package also support to update access_token?) and MS Graph here?