fixup!: review feedback

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

@@ -25,6 +25,7 @@ import org.openedx.app.databinding.ActivityAppBinding
 import org.openedx.app.deeplink.DeepLink
 import org.openedx.auth.presentation.logistration.LogistrationFragment
 import org.openedx.auth.presentation.signin.SignInFragment
+import org.openedx.core.ApiConstants
 import org.openedx.core.data.storage.CorePreferences
 import org.openedx.core.presentation.global.InsetHolder
 import org.openedx.core.presentation.global.WindowSizeHolder
@@ -68,8 +69,8 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder {
     private val authCode: String?
         get() {
             val data = intent?.data
-            if (data is Uri && data.scheme == BuildConfig.APPLICATION_ID && data.host == "oauth2Callback") {
-                return data.getQueryParameter("code")
+            if (data is Uri && data.scheme == BuildConfig.APPLICATION_ID && data.host == ApiConstants.BrowserLogin.REDIRECT_HOST) {
+                return data.getQueryParameter(ApiConstants.BrowserLogin.CODE_QUERY_PARAM)
             }
             return null
         }
@@ -151,14 +152,11 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder {
             when {
                 corePreferencesManager.user == null -> {
                     val authCode = authCode;
+
                     if (viewModel.isLogistrationEnabled && authCode == null) {
                         addFragment(LogistrationFragment())
                     } else {
-                        val bundle = Bundle()
-                        bundle.putString("auth_code", authCode)
-                        val fragment = SignInFragment()
-                        fragment.arguments = bundle
-                        addFragment(fragment)
+                        addFragment(SignInFragment.newInstance(null, null, authCode = authCode))
                     }
                 }
 
@@ -208,6 +206,10 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder {
         super.onNewIntent(intent)
         this.intent = intent
 
+        if (authCode != null) {
+            addFragment(SignInFragment.newInstance(null, null, authCode = authCode))
+        }
+
         val extras = intent?.extras
         if (extras?.containsKey(DeepLink.Keys.NOTIFICATION_TYPE.value) == true) {
             handlePushNotification(extras)

app/src/main/java/org/openedx/app/di/ScreenModule.kt

@@ -104,7 +104,7 @@ val screenModule = module {
         )
     }
 
-    viewModel { (courseId: String?, infoType: String?) ->
+    viewModel { (courseId: String?, infoType: String?, authCode:String) ->
         SignInViewModel(
             get(),
             get(),
@@ -122,6 +122,7 @@ val screenModule = module {
             get(),
             courseId,
             infoType,
+            authCode,
         )
     }
 

auth/src/main/java/org/openedx/auth/data/api/AuthApi.kt

@@ -38,7 +38,9 @@ interface AuthApi {
         @Field("grant_type") grantType: String,
         @Field("client_id") clientId: String,
         @Field("code") code: String,
-        @Field("redirect_uri") redirectUri: String
+        @Field("redirect_uri") redirectUri: String,
+        @Field("token_type") tokenType: String,
+        @Field("asymmetric_jwt") isAsymmetricJwt: Boolean = true,
     ): AuthResponse
 
     @FormUrlEncoded

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

@@ -48,7 +48,8 @@ class AuthRepository(
             grantType = ApiConstants.GRANT_TYPE_CODE,
             clientId = config.getOAuthClientId(),
             code = code,
-            redirectUri = "${config.getApplicationID()}://oauth2Callback"
+            redirectUri = "${config.getAppId()}://${ApiConstants.BrowserLogin.REDIRECT_HOST}",
+            tokenType = config.getAccessTokenType(),
         ).mapToDomain().processAuthResponse()
     }
 

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

@@ -42,6 +42,7 @@ import androidx.fragment.app.Fragment
 import org.koin.androidx.viewmodel.ext.android.viewModel
 import org.koin.core.parameter.parametersOf
 import org.openedx.auth.R
+import org.openedx.core.ApiConstants
 import org.openedx.core.ui.AuthButtonsPanel
 import org.openedx.core.ui.SearchBar
 import org.openedx.core.ui.displayCutoutForLandscape
@@ -80,7 +81,7 @@ class LogistrationFragment : Fragment() {
                             UrlUtils.openInBrowser(
                                 activity = context,
                                 apiHostUrl = viewModel.apiHostUrl,
-                                url = "/register",
+                                url = ApiConstants.URL_REGISTER_BROWSER,
                             )
                         } else {
                             viewModel.navigateToSignUp(parentFragmentManager)

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

@@ -25,7 +25,8 @@ class SignInFragment : Fragment() {
     private val viewModel: SignInViewModel by viewModel {
         parametersOf(
             requireArguments().getString(ARG_COURSE_ID, ""),
-            requireArguments().getString(ARG_INFO_TYPE, "")
+            requireArguments().getString(ARG_INFO_TYPE, ""),
+            requireArguments().getString(ARG_AUTH_CODE, ""),
         )
     }
 
@@ -43,10 +44,8 @@ class SignInFragment : Fragment() {
                 val appUpgradeEvent by viewModel.appUpgradeEvent.observeAsState(null)
 
                 if (appUpgradeEvent == null) {
-                    val authCode = arguments?.getString("auth_code")
-                    if (authCode is String && !state.loginFailure && !state.loginSuccess) {
-                        arguments?.remove("auth_code")
-                        viewModel.signInAuthCode(authCode)
+                    if (viewModel.authCode != "" && !state.loginFailure && !state.loginSuccess) {
+                        viewModel.signInAuthCode(viewModel.authCode)
                     }
                     LoginScreen(
                         windowSize = windowSize,
@@ -101,11 +100,13 @@ class SignInFragment : Fragment() {
     companion object {
         private const val ARG_COURSE_ID = "courseId"
         private const val ARG_INFO_TYPE = "info_type"
-        fun newInstance(courseId: String?, infoType: String?): SignInFragment {
+        private const val ARG_AUTH_CODE = "auth_code"
+        fun newInstance(courseId: String?, infoType: String?, authCode: String? = null): SignInFragment {
             val fragment = SignInFragment()
             fragment.arguments = bundleOf(
                 ARG_COURSE_ID to courseId,
-                ARG_INFO_TYPE to infoType
+                ARG_INFO_TYPE to infoType,
+                ARG_AUTH_CODE to authCode,
             )
             return fragment
         }

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

@@ -59,6 +59,7 @@ class SignInViewModel(
     val config: Config,
     val courseId: String?,
     val infoType: String?,
+    val authCode: String,
 ) : BaseViewModel() {
 
     private val logger = Logger("SignInViewModel")
@@ -190,9 +191,9 @@ class SignInViewModel(
                     onUnknownError()
                     _uiState.update { it.copy(loginFailure = true) }
                 }.onSuccess {
-                    logger.d { "Browser login success" }
                     _uiState.update { it.copy(loginSuccess = true) }
                     setUserId()
+                    appNotifier.send(SignInEvent())
                     _uiState.update { it.copy(showProgress = false) }
                 }
         }

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

@@ -234,28 +234,30 @@ private fun AuthForm(
                 description = stringResource(id = R.string.auth_enter_email_username),
                 onValueChanged = {
                     login = it
-                isEmailError = false
-            },
-            isError = isEmailError,
-            errorMessages = stringResource(id = R.string.auth_error_empty_username_email)
-        )
+                    isEmailError = false
+                },
+                isError = isEmailError,
+                errorMessages = stringResource(id = R.string.auth_error_empty_username_email)
+            )
 
             Spacer(modifier = Modifier.height(18.dp))
             PasswordTextField(
                 modifier = Modifier
                     .fillMaxWidth(),
                 onValueChanged = {
                     password = it
-                isPasswordError = false},
-                onPressDone = {keyboardController?.hide()
-                if (password.isNotEmpty()) {
-                    onEvent(AuthEvent.SignIn(login = login, password = password))
-                }else {
-                    isEmailError = login.isEmpty()
-                    isPasswordError = password.isEmpty()
-                }
-            },
-            isError = isPasswordError,
+                    isPasswordError = false
+                },
+                onPressDone = {
+                    keyboardController?.hide()
+                    if (password.isNotEmpty()) {
+                        onEvent(AuthEvent.SignIn(login = login, password = password))
+                    } else {
+                        isEmailError = login.isEmpty()
+                        isPasswordError = password.isEmpty()
+                    }
+                },
+                isError = isPasswordError,
             )
         } else {
             Spacer(modifier = Modifier.height(40.dp))
@@ -270,10 +272,10 @@ private fun AuthForm(
                 if (state.isLogistrationEnabled.not() && state.isRegistrationEnabled) {
                     Text(
                         modifier = Modifier
-                        .testTag("txt_register")
-                        .noRippleClickable {
-                            onEvent(AuthEvent.RegisterClick)
-                        },
+                            .testTag("txt_register")
+                            .noRippleClickable {
+                                onEvent(AuthEvent.RegisterClick)
+                            },
                         text = stringResource(id = coreR.string.core_register),
                         color = MaterialTheme.appColors.primary,
                         style = MaterialTheme.appTypography.labelLarge
@@ -282,10 +284,10 @@ private fun AuthForm(
                 Spacer(modifier = Modifier.weight(1f))
                 Text(
                     modifier = Modifier
-                    .testTag("txt_forgot_password")
-                    .noRippleClickable {
-                        onEvent(AuthEvent.ForgotPasswordClick)
-                    },
+                        .testTag("txt_forgot_password")
+                        .noRippleClickable {
+                            onEvent(AuthEvent.ForgotPasswordClick)
+                        },
                     text = stringResource(id = R.string.auth_forgot_password),
                     color = MaterialTheme.appColors.info_variant,
                     style = MaterialTheme.appTypography.labelLarge
@@ -302,17 +304,16 @@ private fun AuthForm(
                 textColor = MaterialTheme.appColors.primaryButtonText,
                 backgroundColor = MaterialTheme.appColors.secondaryButtonBackground,
                 onClick = {
-                    if(state.isBrowserLoginEnabled) {
+                    if (state.isBrowserLoginEnabled) {
                         onEvent(AuthEvent.SignInBrowser)
                     } else {
-                    keyboardController?.hide()
-                    if (login.isNotEmpty() && password.isNotEmpty()) {
-                        onEvent(AuthEvent.SignIn(login = login, password = password))
-                    } else {
-                        isEmailError = login.isEmpty()
-                        isPasswordError = password.isEmpty()
-                    }
-
+                        keyboardController?.hide()
+                        if (login.isNotEmpty() && password.isNotEmpty()) {
+                            onEvent(AuthEvent.SignIn(login = login, password = password))
+                        } else {
+                            isEmailError = login.isEmpty()
+                            isPasswordError = password.isEmpty()
+                        }
                     }
                 }
             )

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

@@ -5,6 +5,7 @@ import android.content.Intent.FLAG_ACTIVITY_NEW_TASK
 import android.net.Uri
 import androidx.annotation.WorkerThread
 import androidx.browser.customtabs.CustomTabsIntent
+import org.openedx.core.ApiConstants
 import org.openedx.core.config.Config
 import org.openedx.core.utils.Logger
 
@@ -15,10 +16,10 @@ class BrowserAuthHelper(private val config: Config) {
     @WorkerThread
     suspend fun signIn(activityContext: Activity) {
         logger.d { "Browser-based auth initiated" }
-        val uri = Uri.parse("${config.getApiHostURL()}/oauth2/authorize").buildUpon()
+        val uri = Uri.parse("${config.getApiHostURL()}${ApiConstants.URL_AUTHORIZE}").buildUpon()
             .appendQueryParameter("client_id", config.getOAuthClientId())
-            .appendQueryParameter("redirect_uri", "${activityContext.packageName}://oauth2Callback")
-            .appendQueryParameter("response_type", "code").build()
+            .appendQueryParameter("redirect_uri", "${activityContext.packageName}://${ApiConstants.BrowserLogin.REDIRECT_HOST}")
+            .appendQueryParameter("response_type", ApiConstants.BrowserLogin.RESPONSE_TYPE).build()
         val intent =
             CustomTabsIntent.Builder().setUrlBarHidingEnabled(true).setShowTitle(true).build()
         intent.intent.setFlags(FLAG_ACTIVITY_NEW_TASK)

core/src/main/java/org/openedx/core/ApiConstants.kt

@@ -2,13 +2,15 @@ package org.openedx.core
 
 object ApiConstants {
     const val URL_LOGIN = "/oauth2/login/"
+    const val URL_AUTHORIZE = "/oauth2/authorize/"
     const val URL_ACCESS_TOKEN = "/oauth2/access_token/"
     const val URL_EXCHANGE_TOKEN = "/oauth2/exchange_access_token/{auth_type}/"
     const val GET_USER_PROFILE = "/api/mobile/v0.5/my_user_info"
     const val URL_REVOKE_TOKEN = "/oauth2/revoke_token/"
     const val URL_REGISTRATION_FIELDS = "/user_api/v1/account/registration"
     const val URL_VALIDATE_REGISTRATION_FIELDS = "/api/user/v1/validation/registration"
     const val URL_REGISTER = "/api/user/v1/account/registration/"
+    const val URL_REGISTER_BROWSER = "/register"
     const val URL_PASSWORD_RESET = "/password_reset/"
 
     const val GRANT_TYPE_PASSWORD = "password"
@@ -19,6 +21,7 @@ object ApiConstants {
     const val TOKEN_TYPE_REFRESH = "refresh_token"
 
     const val ACCESS_TOKEN = "access_token"
+
     const val CLIENT_ID = "client_id"
     const val EMAIL = "email"
     const val NAME = "name"
@@ -36,4 +39,10 @@ object ApiConstants {
         const val HONOR_CODE = "honor_code"
         const val MARKETING_EMAILS = "marketing_emails_opt_in"
     }
+
+    object BrowserLogin {
+        const val REDIRECT_HOST = "oauth2Callback"
+        const val CODE_QUERY_PARAM = "code"
+        const val RESPONSE_TYPE = "code"
+    }
 }

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

@@ -30,10 +30,6 @@ class Config(context: Context) {
         return getString(URI_SCHEME)
     }
 
-    fun getApplicationID(): String {
-        return getString(APPLICATION_ID, "")
-    }
-
     fun getOAuthClientId(): String {
         return getString(OAUTH_CLIENT_ID)
     }

default_config/dev/config.yaml

@@ -78,7 +78,7 @@ SOCIAL_AUTH_ENABLED: false
 #feature flag to enable registration from app
 REGISTRATION_ENABLED: true
 #feature flag to do the authentication flow in the browser to log in
-BROWSER_LOGIN: false
+BROWSER_LOGIN: true
 #feature flag to do the registration for in the browser
 BROWSER_REGISTRATION: false
 #Course navigation feature flags

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

@@ -1,5 +1,5 @@
-How to user Browser-based Login and Registration
-================================================
+How to use Browser-based Login and Registration
+===============================================
 
 Introduction
 ------------
@@ -14,7 +14,8 @@ the application.
 
 The ``BROWSER_REGISTRATION`` flag is used to redirect registration to the browser. In this case
 clicking on the registration button will open the registration page in a regular browser tab. Once
-registered, the user will **not** be automatically redirected to the application.
+registered, the user will as of writing this document **not** be automatically redirected to the
+application.
 
 Usage
 -----
@@ -42,4 +43,6 @@ schemes here, including ``"https"`` and ``"http"``.
 
 The authentication will then redirect to the browser in a custom tab that redirects back to the app.
 
-NOTE: If a user logs out from the application, they might still be logged in, in the browser.
+..note::
+
+    If a user logs out from the application, they might still be logged in, in the browser.