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

Extend Response of the API #80

Closed
futurbotconsolidated opened this issue Nov 17, 2021 · 14 comments
Closed

Extend Response of the API #80

futurbotconsolidated opened this issue Nov 17, 2021 · 14 comments
Labels
enhancement New feature or request

Comments

@futurbotconsolidated
Copy link

How can I extend the response of the api
for eg: Add User Data in it

@futurbotconsolidated futurbotconsolidated changed the title Extend Response Extend Response of the API Nov 17, 2021
@wagnerdelima
Copy link
Owner

wagnerdelima commented Dec 23, 2021

Can you describe your needs in a more detailed manner? What exactly do you need? Please use images to better describe your needs.

@wagnerdelima
Copy link
Owner

Hi @futurbotconsolidated,

Can you be more descriptive here, please?

@futurbotconsolidated
Copy link
Author

futurbotconsolidated commented Dec 24, 2021

@wagnerdelima As now, We are getting only 4 data attributes which are access_token,Refresh_token, Expiry and one more. But extend to this, while we login, It would be better if could get user data like username,firstname, profile_picture or any other data we require in the response. Since now we are sending another API request for the particular data after recieving the token. It would be better if we get everything from a single request. (#49 had the same query).

Also, I would like to request for a feature which is
" Returning the same token if not expired or present"

Everytime we hit the url, it creates new access and refresh token and gives us the response, It would be a better approach if it should return the same tokens respective to user until it is not expired.

@wagnerdelima
Copy link
Owner

@futurbotconsolidated, I liked your idea: "Returning the same token if not expired or present". I will investigate the extension of the data response.

Happy xmas mate.

@wagnerdelima
Copy link
Owner

wagnerdelima commented Dec 26, 2021

@futurbotconsolidated

I found where the tokens are being recreated. However, it's not created by drf_social_oauth2. It's created by django_social_auth. AccessTokens are created here: https://github.com/jazzband/django-oauth-toolkit/blob/461061687b59acfa5837cda092b60c5ed85f385e/oauth2_provider/oauth2_validators.py#L605.

RefreshTokens are created here: https://github.com/jazzband/django-oauth-toolkit/blob/461061687b59acfa5837cda092b60c5ed85f385e/oauth2_provider/oauth2_validators.py#L635.

I will submit a pull request on django_social_auth to solve this issue. In case it's not accepted I will have to tackle it at drf_social_oauth2 level. Thing is, if drf tackled it, we would have to make multiple queries at database level, which I want to aoiv, obviously.

Draft PR created at: jazzband/django-oauth-toolkit#1058
Thanks for your suggestion.

@futurbotconsolidated
Copy link
Author

@wagnerdelima Yeah, I have checked it too. I read most of the code of drf and social auth Also I have suggested this because google token system works this way too.

Also, I would like to add here if the developers could get more easy access to login and signup class. So they could edit the request and response accordingly.
For example, if we take a case, There are 2 types of users- 1. Men users 2. Female Users
and we want to give the login access to anyone of them only. Let's say only male users

Then when we login, we should be able to check the gender type and return the suitable response. But in here, It is all inside the library quite difficult to moderate. So it would be better if we allow developers to write their own view class.

I will be forking drf-social-oauth 2 in the next coming days and will try to implement some of my suggestions.
Thanks for this amazing rep. Cheeers!

@wagnerdelima
Copy link
Owner

@futurbotconsolidated thank you for the compliment.

We could discuss here some business logic to add. If we entangled drf-social-oauth2 with too much of yours or my own business, it kinda of stops being a generic lib and becomes someones personal repo.

I am very curious about your changes though. Please send them over.

@wagnerdelima
Copy link
Owner

@futurbotconsolidated I have a question for you.
Did you find integrating this framework too difficult?

A few people are opening issues with vary vague questions, such as: what is google code?, etc.

@futurbotconsolidated
Copy link
Author

@wagnerdelima No, It's very easy to implement until you don't have to make major changes. Trust me it took me only 5 mins to set up everything.

I think people who are asking questions like these are not much familiar with django framework because you don't have to study other libraries if you have basic needs. This framework contains all.

I feel these people don't study the concepts, they just believe in watching the tutorial and copy pasting the code. That is why they just ask vague questions and expect author to answer.

@wagnerdelima
Copy link
Owner

Thanks for your feedback @futurbotconsolidated.

@wagnerdelima
Copy link
Owner

@futurbotconsolidated, the same token issue is resolved. Thanks for the suggestion.

@SukiCZ
Copy link

SukiCZ commented Sep 14, 2022

TL;DR: Could we please revert the changes that have been made in the feat/remove-token-duplication branch?

Hi @wagnerdelima ,

I'm having some difficulties with GitHub OAuth implementation as mentioned in #56
GitHub only allows to use a Authorization Code Grand with a token that can be only used once. The method SocialTokenServer.create_token_response obtains user information and query database for existing Access Token. If not found, it will construct second request with Authorization Code which was already used and the process fails.

I understand that you were trying to mitigate the issue related to re-creating tokens as mentioned by @futurbotconsolidated , but I strongly believe that when the client sends a request to convert-token a new token should be issued. Both Refresh and Access token have configurable EXPIRY and ./manage.py cleartokens command is available to clear the old ones - similar to clear_sessions.

I'm available to provide Pull Request if you agree with my rationale.

P.S. Thanks for the maintenance of the REST library. It's all fun until you have to debug OAuth 😅

@wagnerdelima
Copy link
Owner

@SukiCZ I made a comment to the other issue regarding github login. Let's get this sorted first then we can talk about reverting the changes from feat/remove-token-duplication.

@wagnerdelima
Copy link
Owner

@futurbotconsolidated I am closing this issue. This seemed like a good idea at first. I have been thinking about this for a while now but I think it would be unfeasible to extend api response. Every social backend has it's own response type, this way, it would be almost impossible to make this work without constant interruptions and changes.

I am happy to review any PR you may raise on this topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

3 participants