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

Feature/Add social login via Github and Active Directory #75

Merged
merged 4 commits into from Jan 28, 2019

Conversation

c-w
Copy link
Member

@c-w c-w commented Jan 25, 2019

Currently the workflow for adding new users to a doccano instance requires an administrator to create accounts using the Django admin tool. This puts a burden on the admin. It also puts a burden on the user since they must maintain a dedicated account and password to access this application. See also the discussion on card-16405801 around user management.

To make it easier for annotators to do work on doccano, I suggest to add social authentication to the application. This means that a user can sign up with the social provider and get invited to work on annotation projects using that social identity.

As a first step, this pull request implements Github and Azure Active Directory authentication but other social identities should be easily added in the future. This pull request does not solve the problem of having an easy creation of admin users who can create new projects, but at least it makes it simple for annotators to join and work on a project.

Screenshot of the updated login page:

Login page screenshot

@c-w c-w changed the title Feature/Add social login via Github Feature/Add social login via Github and Active Directory Jan 25, 2019
@Hironsan
Copy link
Member

@Hironsan Hironsan commented Jan 28, 2019

Nice feature!
But I have a question.

In urls.py, is the following url right?

I tried to login vie GitHub OAuth, then I had an Authentication Failed error:
image

According to the Python Social Auth documentation, I changed the url from above to below. It works fine:

# path('social/', include('social_django.urls')),
path(r'', include('social_django.urls', namespace='social')),

Do we need any additional settings?

@c-w
Copy link
Member Author

@c-w c-w commented Jan 28, 2019

@Hironsan Thanks for your review!

Unlike the social_django documentation, I've used path('social/', include('social_django.urls')), in this pull request to ensure that none of the included URLs from social_django will ever clash with URLs defined in the main application since setting the first argument of the url function to social/ means that all the social_django URLs will be prefixed with that token.

So for example the Github complete URL will now be /social/complete/github instead of /complete/github. This simply requires the callback URL in the Github OAuth application to be configured slightly differently, to include the /social prefix. See the screenshot below for an example:

Screenshot of Github OAuth application registration

I believe that having the social/ prefix has benefits since it means that developers of doccano don't have to be aware of all the potential URLs in the social_django project and we don't get odd bugs in the future of routes overriding each other.

Could you try with the branch's code and change the Github callback URL in your OAuth application registration to include the social/ prefix and confirm that the feature then works for you? Thanks in advance!

@Hironsan
Copy link
Member

@Hironsan Hironsan commented Jan 28, 2019

I believe that having the social/ prefix has benefits since it means that developers of doccano don't have to be aware of all the potential URLs in the social_django project and we don't get odd bugs in the future of routes overriding each other.

I agree with you.

It works fine. Thank you!

@Hironsan Hironsan merged commit 537d1ca into doccano:master Jan 28, 2019
2 checks passed
@Hironsan
Copy link
Member

@Hironsan Hironsan commented Jan 28, 2019

@BrambleXu BrambleXu added the feature request label Jan 28, 2019
@BrambleXu BrambleXu added this to PR Status in v1.0.0 Jan 28, 2019
@c-w c-w deleted the feature/social branch Jan 28, 2019
@c-w c-w mentioned this pull request Feb 19, 2020
@setu4993 setu4993 mentioned this pull request Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request
Projects
No open projects
v1.0.0
PR Status
Development

Successfully merging this pull request may close these issues.

None yet

3 participants