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

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

Open
aoiheaven opened this issue Dec 14, 2022 · 8 comments

Comments

@aoiheaven
Copy link

aoiheaven commented Dec 14, 2022

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?

code = request.GET.get("code")

return redirect(redirect_to)

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

Fund with Polar
@JonasKs
Copy link
Member

JonasKs commented Dec 14, 2022

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
Copy link
Member

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
Copy link
Member

JonasKs commented Dec 14, 2022

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

@tim-schilling
Copy link
Member

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

@JonasKs
Copy link
Member

JonasKs commented Dec 14, 2022

Ohhh, you're right! Sorry.

@tim-schilling
Copy link
Member

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

@aoiheaven
Copy link
Author

aoiheaven commented Dec 14, 2022

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
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants