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

xitij2000 · 2024-07-30T16:54:08Z

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-07-30T16:54:13Z

Thanks for the pull request, @xitij2000!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/openedx-mobile-maintainers. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

navinkarkera

@xitij2000 Nice work! 👍

👍

I tested this: (tested login in both real hardware as well as emulator)
I read through the code
I checked for accessibility issues
Includes documentation

mphilbrick211 · 2024-10-24T21:01:12Z

@openedx/openedx-mobile-maintainers - is someone able to please take a look at this?

volodymyr-chekyrta · 2024-11-04T10:26:04Z

@openedx/openedx-mobile-maintainers - is someone able to please take a look at this?

I'll review it 👀
@xitij2000, could you change the target branch to develop and fix conflicts if they appear?
Thank you!

xitij2000 · 2024-11-04T11:12:40Z

@openedx/openedx-mobile-maintainers - is someone able to please take a look at this?

I'll review it 👀 @xitij2000, could you change the target branch to develop and fix conflicts if they appear? Thank you!

@volodymyr-chekyrta I've changed the target branch; however, I've had some issues running the develop branch before due to it often being reliant on unmerged PRs in edx-platform. So I might need your help with that.

Could you have a quick look and see if the general direction of the PR is okay or if bigger changes are needed so I can budget the time accordingly?

volodymyr-chekyrta · 2024-11-05T19:45:18Z

@openedx/openedx-mobile-maintainers - is someone able to please take a look at this?

I'll review it 👀 @xitij2000, could you change the target branch to develop and fix conflicts if they appear? Thank you!

@volodymyr-chekyrta I've changed the target branch; however, I've had some issues running the develop branch before due to it often being reliant on unmerged PRs in edx-platform. So I might need your help with that.

Could you have a quick look and see if the general direction of the PR is okay or if bigger changes are needed so I can budget the time accordingly?

@xitij2000 I checked the approach, and it is good 👍; I have a few comments according to the code, but it will be better to address them after resolving conflicts

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-11-06T06:42:23Z

@openedx/openedx-mobile-maintainers - is someone able to please take a look at this?

I'll review it 👀 @xitij2000, could you change the target branch to develop and fix conflicts if they appear? Thank you!

@volodymyr-chekyrta I've changed the target branch; however, I've had some issues running the develop branch before due to it often being reliant on unmerged PRs in edx-platform. So I might need your help with that.
Could you have a quick look and see if the general direction of the PR is okay or if bigger changes are needed so I can budget the time accordingly?

@xitij2000 I checked the approach, and it is good 👍; I have a few comments according to the code, but it will be better to address them after resolving conflicts

I've rebased the code, but still need to test that everything works.

volodymyr-chekyrta

Minor changes, mostly move value to constant:

volodymyr-chekyrta · 2024-11-06T08:28:57Z

app/src/main/java/org/openedx/app/AppActivity.kt

+            if (data is Uri && data.scheme == BuildConfig.APPLICATION_ID && data.host == "oauth2Callback") {
+                return data.getQueryParameter("code")
+            }


Please move "oauth2Callback" and "code" to constants.

volodymyr-chekyrta · 2024-11-06T08:29:58Z

app/src/main/java/org/openedx/app/AppActivity.kt

+                        val bundle = Bundle()
+                        bundle.putString("auth_code", authCode)
+                        val fragment = SignInFragment()
+                        fragment.arguments = bundle
+                        addFragment(fragment)


Please use SignInFragment.newIntance(...) method

volodymyr-chekyrta · 2024-11-06T08:30:19Z

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

+            grantType = ApiConstants.GRANT_TYPE_CODE,
+            clientId = config.getOAuthClientId(),
+            code = code,
+            redirectUri = "${config.getApplicationID()}://oauth2Callback"


oauth2Callback to constants

volodymyr-chekyrta · 2024-11-06T08:30:44Z

auth/src/main/java/org/openedx/auth/presentation/logistration/LogistrationFragment.kt

+                            UrlUtils.openInBrowser(
+                                activity = context,
+                                apiHostUrl = viewModel.apiHostUrl,
+                                url = "/register",


Please move /register to constants

volodymyr-chekyrta · 2024-11-06T08:31:55Z

auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt

+                    val authCode = arguments?.getString("auth_code")
+                    if (authCode is String && !state.loginFailure && !state.loginSuccess) {
+                        arguments?.remove("auth_code")


Please move arguments like "auth_code" to companion object constants.
e.g.

companion object { private const val ARG_COURSE_ID = "courseId" private const val ARG_INFO_TYPE = "info_type" private const val ARG_AUTH_CODE = "auth_code" fun newInstance(courseId: String?, infoType: String?): SignInFragment { val fragment = SignInFragment() fragment.arguments = bundleOf( ARG_COURSE_ID to courseId, ARG_INFO_TYPE to infoType ) return fragment } }

volodymyr-chekyrta · 2024-11-06T08:34:54Z

auth/src/main/java/org/openedx/auth/presentation/signin/compose/SignInView.kt

+                    if(state.isBrowserLoginEnabled) {
+                        onEvent(AuthEvent.SignInBrowser)


if(state.isBrowserLoginEnabled) -> if (state.isBrowserLoginEnabled)

volodymyr-chekyrta · 2024-11-06T08:35:33Z

auth/src/main/java/org/openedx/auth/presentation/signin/compose/SignInView.kt

+                    } else {
                    keyboardController?.hide()


Suggested change

} else {

keyboardController?.hide()

} else {

keyboardController?.hide()

volodymyr-chekyrta · 2024-11-06T08:36:51Z

auth/src/main/java/org/openedx/auth/presentation/sso/BrowserAuthHelper.kt

+            .appendQueryParameter("client_id", config.getOAuthClientId())
+            .appendQueryParameter("redirect_uri", "${activityContext.packageName}://oauth2Callback")
+            .appendQueryParameter("response_type", "code").build()


It probably will be good to move the params to constants as well

volodymyr-chekyrta · 2024-11-06T08:38:22Z

core/src/main/java/org/openedx/core/config/Config.kt

+    fun getApplicationID(): String {
+        return getString(APPLICATION_ID, "")
+    }
+


duplicates:

fun getAppId(): String { return getString(APPLICATION_ID, "") }

volodymyr-chekyrta · 2024-11-06T08:40:02Z

docs/how-tos/auth-using-browser.rst

+How to user Browser-based Login and Registration
+================================================
+


Just wanted to point out that I like this doc 👍

xitij2000 · 2024-11-07T09:08:24Z

@volodymyr-chekyrta I will allocate some time soon to go through these properly and update the PR. Thanks for the great feedback!

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

itsjeyd mentioned this pull request Aug 1, 2024

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

Open

1 task

navinkarkera approved these changes Aug 5, 2024

View reviewed changes

xitij2000 marked this pull request as ready for review August 13, 2024 13:51

This was referenced Aug 13, 2024

feat: Allow trusted apps to perform cookie login. openedx/edx-platform#35221

Merged

feat: Allow trusted apps to perform cookie login. open-craft/edx-platform#668

Merged

mphilbrick211 requested a review from a team August 22, 2024 20:39

navinkarkera mentioned this pull request Sep 13, 2024

feat: Support for login and registration via a browser custom tab open-craft/openedx-app-android#2

Merged

xitij2000 changed the base branch from main to develop November 4, 2024 11:01

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

92cb74e

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 b0b4165 to 92cb74e Compare November 6, 2024 06:41

volodymyr-chekyrta requested changes Nov 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

xitij2000 commented Jul 30, 2024 •

edited

Loading

openedx-webhooks commented Jul 30, 2024 •

edited

Loading

navinkarkera left a comment

mphilbrick211 commented Oct 24, 2024

volodymyr-chekyrta commented Nov 4, 2024

xitij2000 commented Nov 4, 2024

volodymyr-chekyrta commented Nov 5, 2024

xitij2000 commented Nov 6, 2024

volodymyr-chekyrta left a comment

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

volodymyr-chekyrta Nov 6, 2024

xitij2000 commented Nov 7, 2024

		if(state.isBrowserLoginEnabled) {
		onEvent(AuthEvent.SignInBrowser)

		How to user Browser-based Login and Registration
		================================================

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

Are you sure you want to change the base?

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

Conversation

xitij2000 commented Jul 30, 2024 • edited Loading

openedx-webhooks commented Jul 30, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

navinkarkera left a comment

Choose a reason for hiding this comment

mphilbrick211 commented Oct 24, 2024

volodymyr-chekyrta commented Nov 4, 2024

xitij2000 commented Nov 4, 2024

volodymyr-chekyrta commented Nov 5, 2024

xitij2000 commented Nov 6, 2024

volodymyr-chekyrta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xitij2000 commented Nov 7, 2024

xitij2000 commented Jul 30, 2024 •

edited

Loading

openedx-webhooks commented Jul 30, 2024 •

edited

Loading