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.

Sign up for GitHub

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

Jump to bottom

IPC on behalf of user #10

Merged
merged 15 commits into from
Dec 9, 2019
Merged

IPC on behalf of user #10

merged 15 commits into from
Dec 9, 2019

Conversation

ArmaanT
Copy link
Member

@ArmaanT ArmaanT commented Nov 11, 2019 •
edited
Loading

This PR sets up IPC on behalf of the user. It consists of:

  • Storing the user's access and refresh tokens in a model linked to the user
  • A utility function to make authenticated requests using the user's access token
  • A middleware to receive a user's access token and set the request.user appropriately

[ch97]
[ch157]
[ch284]

@ArmaanT ArmaanT force-pushed the feature/ipc branch 2 times, most recently from 84f055b to 0f72746 Compare November 25, 2019 18:50
@coveralls
Copy link

coveralls commented Nov 25, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 3191c1a on feature/ipc into 8dc6dbc on master.

@ArmaanT ArmaanT force-pushed the feature/ipc branch 3 times, most recently from e9671a7 to 5a4b8fd Compare November 25, 2019 19:18
@ArmaanT ArmaanT changed the title [WIP] IPC on behalf of user IPC on behalf of user Nov 27, 2019
@ArmaanT ArmaanT requested a review from davish November 27, 2019 22:03
davish
davish requested changes Nov 27, 2019
View reviewed changes
Copy link
Member

@davish davish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge pennlabs/template-django/pull/4

@ArmaanT ArmaanT requested a review from Rfax November 27, 2019 22:31
davish
davish requested changes Nov 27, 2019
View reviewed changes
Copy link
Member

@davish davish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nitpicks and naming/documentation questions, but overall looks really good! This is exciting. There are a few comments in there that might lead to more discussion.

README.md Show resolved Hide resolved
accounts/backends.py Outdated Show resolved Hide resolved
accounts/backends.py Outdated Show resolved Hide resolved
accounts/middleware.py Outdated Show resolved Hide resolved
accounts/middleware.py Show resolved Hide resolved
accounts/utils.py Outdated Show resolved Hide resolved
accounts/utils.py Outdated Show resolved Hide resolved
accounts/utils.py Outdated Show resolved Hide resolved
accounts/utils.py Outdated Show resolved Hide resolved
accounts/utils.py Outdated Show resolved Hide resolved
@ArmaanT ArmaanT requested a review from davish November 28, 2019 02:39
davish
davish approved these changes Nov 29, 2019
View reviewed changes
Copy link
Member

@davish davish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just throwing a 403 in that one spot to be consistent.

accounts/ipc.py Show resolved Hide resolved
@ArmaanT ArmaanT merged commit 361c0c6 into master Dec 9, 2019
@ArmaanT ArmaanT deleted the feature/ipc branch December 9, 2019 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davish davish davish approved these changes

@Rfax Rfax Awaiting requested review from Rfax

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ArmaanT @coveralls @davish