feat: Support for login and registration via a browser custom tab by xitij2000 · Pull Request #190 · openedx/openedx-app-android

xitij2000 · 2024-01-18T07:04:51Z

This change adds support for logging in and registering a new account using the browser. This can be useful for cases where the only way to log into the instatance is via a custom third-party auth provider.

openedx-webhooks · 2024-01-18T07:04:56Z

Thanks for the pull request, @xitij2000! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

yusuf-musleh

@xitij2000 Great work on this! It's working quite well when I tested it out. I just had a small comment on some debug code pushed. I think it would also be helpful to add some documentation/instructions either in the README or the Documentation.md about how to enable/setup the browser based authentication.

I tested this: (Configured and built the app on my phone with browser based auth with @xitij2000 server running the LMS/CMS, and tested registration/login flows)
I read through the code
~~I checked for accessibility issues~~
Includes documentation: Missing but flagged it
~~I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.~~

yusuf-musleh · 2024-01-21T10:32:58Z

auth/src/main/java/org/openedx/auth/data/repository/AuthRepository.kt

    }

    private suspend fun AuthResponse.processAuthResponse() {
+        Log.d("MMMMMMMM", error.toString())


Seems like some debug code here

yusuf-musleh · 2024-01-21T10:33:26Z

auth/src/main/java/org/openedx/auth/data/repository/AuthRepository.kt

+        Log.d("MMMMMMMM", preferencesManager.accessToken)
+        Log.d("MMMMMMMM", preferencesManager.refreshToken)
+        Log.d("MMMMMMMM", preferencesManager.accessTokenExpiresAt.toString())


Similarly some debugging code here

This change adds support for logging in and registering a new account using the browser. This can be useful for cases where the only way to log into the instatance is via a custom third-party auth provider.

xitij2000 · 2024-01-22T08:56:05Z

@yusuf-musleh I've updated the PR to remove the debugging logs.
As for the documentation, I don't see much other relevant documentation in the readme or documentation.md, but will add a new document.

yusuf-musleh · 2024-01-22T17:21:00Z

@xitij2000 The new doc looks good to me! 👍

mphilbrick211 · 2024-03-20T01:53:22Z

Hi @xitij2000! Just checking in on this.

xitij2000 · 2024-03-22T09:49:38Z

Hi @xitij2000! Just checking in on this.

I'd love some feedback on the approach etc. If that seems OK I can update the PR and get it ready to merge.

marcotuts · 2024-04-03T15:19:16Z

A few notes @xitij2000 on my end - This is good to go on the product review side, though I would suggest that this be set up as a configurable feature flag defaulted to being off. This will keep the native login / register as the default in the mobile applications.

Additionally, there may be some need for conflict resolution and rebasing for the review to continue. @volodymyr-chekyrta can review again once these items are addressed (and any other items as well that came up in the review above that may be outstanding. )

thanks!

marcotuts

Product review only 👍

xitij2000 · 2024-04-04T12:07:36Z

A few notes @xitij2000 on my end - This is good to go on the product review side, though I would suggest that this be set up as a configurable feature flag defaulted to being off. This will keep the native login / register as the default in the mobile applications.

Additionally, there may be some need for conflict resolution and rebasing for the review to continue. @volodymyr-chekyrta can review again once these items are addressed (and any other items as well that came up in the review above that may be outstanding. )

thanks!

Thanks for the review! The intention is definitely to keep the current login experience intact, and just allow this alternative flow for cases where it's needed.

volodymyr-chekyrta · 2024-04-08T14:06:42Z

@xitij2000, could you please resolve conflicts so I can proceed with review?

xitij2000 · 2024-04-12T07:27:47Z

@xitij2000, could you please resolve conflicts so I can proceed with review?

This PR is in the similar position as openedx/openedx-app-ios#65 (comment)

I've tried rebasing but need more time that I have currently, since the rebased version is nto working.

openedx-webhooks · 2024-05-21T10:54:46Z

@xitij2000 Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 18, 2024

xitij2000 force-pushed the kshitij/custom-auth-2 branch 2 times, most recently from 389f90f to dc9023c Compare January 19, 2024 15:21

yusuf-musleh approved these changes Jan 21, 2024

View reviewed changes

xitij2000 force-pushed the kshitij/custom-auth-2 branch from dc9023c to 530e8c5 Compare January 22, 2024 08:29

feat: Support for login and registration via a browser custom tab

b0b7bfb

This change adds support for logging in and registering a new account using the browser. This can be useful for cases where the only way to log into the instatance is via a custom third-party auth provider.

xitij2000 force-pushed the kshitij/custom-auth-2 branch from 530e8c5 to b0b7bfb Compare January 22, 2024 08:44

xitij2000 marked this pull request as ready for review January 22, 2024 08:45

xitij2000 mentioned this pull request Jan 30, 2024

feat: Add support for browser-based logins #45

Closed

docs: Add documentation for the browser auth feature

2790805

xitij2000 force-pushed the kshitij/custom-auth-2 branch from 7fcd757 to 2790805 Compare February 7, 2024 15:19

marcotuts self-requested a review April 3, 2024 15:19

marcotuts approved these changes Apr 3, 2024

View reviewed changes

itsjeyd mentioned this pull request Apr 25, 2024

[Product Pull Request] feat: Support web-based LMS OAuth openedx/platform-roadmap#345

Closed

1 task

mphilbrick211 added the product review PR requires product review before merging label May 1, 2024

xitij2000 closed this May 21, 2024

Conversation

xitij2000 commented Jan 18, 2024

Uh oh!

openedx-webhooks commented Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yusuf-musleh left a comment

Choose a reason for hiding this comment

Uh oh!

yusuf-musleh Jan 21, 2024

Choose a reason for hiding this comment

Uh oh!

yusuf-musleh Jan 21, 2024

Choose a reason for hiding this comment

Uh oh!

xitij2000 commented Jan 22, 2024

Uh oh!

yusuf-musleh commented Jan 22, 2024

Uh oh!

mphilbrick211 commented Mar 20, 2024

Uh oh!

xitij2000 commented Mar 22, 2024

Uh oh!

marcotuts commented Apr 3, 2024

Uh oh!

marcotuts left a comment

Choose a reason for hiding this comment

Uh oh!

xitij2000 commented Apr 4, 2024

Uh oh!

volodymyr-chekyrta commented Apr 8, 2024

Uh oh!

xitij2000 commented Apr 12, 2024

Uh oh!

openedx-webhooks commented May 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

openedx-webhooks commented Jan 18, 2024 •

edited

Loading