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

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@

<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
<intent-filter>
<action android:name="android.intent.action.VIEW" />
<category android:name="android.intent.category.DEFAULT" />
<category android:name="android.intent.category.BROWSABLE" />
<data android:scheme="${applicationId}" />
</intent-filter>

<!-- Branch URI Scheme -->
<intent-filter>
Expand Down
18 changes: 16 additions & 2 deletions app/src/main/java/org/openedx/app/AppActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.openedx.app
import android.content.Intent
import android.content.res.Configuration
import android.graphics.Color
import android.net.Uri
import android.os.Bundle
import android.view.View
import android.view.WindowManager
Expand Down Expand Up @@ -64,6 +65,14 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder {
private var _insetCutout = 0

private var _windowSize = WindowSize(WindowType.Compact, WindowType.Compact)
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")
}
Comment on lines +71 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

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

return null
}

private val branchCallback =
BranchUniversalReferralInitListener { branchUniversalObject, _, error ->
Expand Down Expand Up @@ -141,10 +150,15 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder {
if (savedInstanceState == null) {
when {
corePreferencesManager.user == null -> {
if (viewModel.isLogistrationEnabled) {
val authCode = authCode;
if (viewModel.isLogistrationEnabled && authCode == null) {
addFragment(LogistrationFragment())
} else {
addFragment(SignInFragment())
val bundle = Bundle()
bundle.putString("auth_code", authCode)
val fragment = SignInFragment()
fragment.arguments = bundle
addFragment(fragment)
Comment on lines +157 to +161
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use SignInFragment.newIntance(...) method

}
}

Expand Down
2 changes: 2 additions & 0 deletions app/src/main/java/org/openedx/app/di/AppModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import org.openedx.app.room.DatabaseManager
import org.openedx.auth.presentation.AgreementProvider
import org.openedx.auth.presentation.AuthAnalytics
import org.openedx.auth.presentation.AuthRouter
import org.openedx.auth.presentation.sso.BrowserAuthHelper
import org.openedx.auth.presentation.sso.FacebookAuthHelper
import org.openedx.auth.presentation.sso.GoogleAuthHelper
import org.openedx.auth.presentation.sso.MicrosoftAuthHelper
Expand Down Expand Up @@ -202,6 +203,7 @@ val appModule = module {
factory { FacebookAuthHelper() }
factory { GoogleAuthHelper(get()) }
factory { MicrosoftAuthHelper() }
factory { BrowserAuthHelper(get()) }
factory { OAuthHelper(get(), get(), get()) }

factory { FileUtil(get(), get<ResourceManager>().getString(R.string.app_name)) }
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/java/org/openedx/app/di/ScreenModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ val screenModule = module {
get(),
get(),
get(),
get(),
)
}

Expand All @@ -118,6 +119,7 @@ val screenModule = module {
get(),
get(),
get(),
get(),
courseId,
infoType,
)
Expand Down
1 change: 1 addition & 0 deletions auth/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ android {
dependencies {
implementation project(path: ':core')

implementation 'androidx.browser:browser:1.7.0'
implementation "androidx.credentials:credentials:1.3.0"
implementation "androidx.credentials:credentials-play-services-auth:1.3.0"
implementation "com.facebook.android:facebook-login:16.2.0"
Expand Down
9 changes: 9 additions & 0 deletions auth/src/main/java/org/openedx/auth/data/api/AuthApi.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ interface AuthApi {
@Field("asymmetric_jwt") isAsymmetricJwt: Boolean = true,
): AuthResponse

@FormUrlEncoded
@POST(ApiConstants.URL_ACCESS_TOKEN)
suspend fun getAccessTokenFromCode(
@Field("grant_type") grantType: String,
@Field("client_id") clientId: String,
@Field("code") code: String,
@Field("redirect_uri") redirectUri: String
): AuthResponse

@FormUrlEncoded
@POST(ApiConstants.URL_ACCESS_TOKEN)
fun refreshAccessToken(
Expand Down
1 change: 1 addition & 0 deletions auth/src/main/java/org/openedx/auth/data/model/AuthType.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ enum class AuthType(val postfix: String, val methodName: String) {
GOOGLE(ApiConstants.AUTH_TYPE_GOOGLE, "Google"),
FACEBOOK(ApiConstants.AUTH_TYPE_FB, "Facebook"),
MICROSOFT(ApiConstants.AUTH_TYPE_MICROSOFT, "Microsoft"),
BROWSER(ApiConstants.AUTH_TYPE_BROWSER, "Browser")
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ class AuthRepository(
.processAuthResponse()
}

suspend fun browserAuthCodeLogin(code: String) {
api.getAccessTokenFromCode(
grantType = ApiConstants.GRANT_TYPE_CODE,
clientId = config.getOAuthClientId(),
code = code,
redirectUri = "${config.getApplicationID()}://oauth2Callback"
Copy link
Contributor

Choose a reason for hiding this comment

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

oauth2Callback to constants

).mapToDomain().processAuthResponse()
}

suspend fun getRegistrationFields(): List<RegistrationField> {
return api.getRegistrationFields().fields?.map { it.mapToDomain() } ?: emptyList()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ class AuthInteractor(private val repository: AuthRepository) {
repository.socialLogin(token, authType)
}

suspend fun loginAuthCode(authCode: String) {
repository.browserAuthCodeLogin(authCode)
}

suspend fun getRegistrationFields(): List<RegistrationField> {
return repository.getRegistrationFields()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import org.openedx.core.ui.theme.OpenEdXTheme
import org.openedx.core.ui.theme.appColors
import org.openedx.core.ui.theme.appTypography
import org.openedx.core.ui.theme.compose.LogistrationLogoView
import org.openedx.foundation.utils.UrlUtils

class LogistrationFragment : Fragment() {

Expand All @@ -67,10 +68,23 @@ class LogistrationFragment : Fragment() {
OpenEdXTheme {
LogistrationScreen(
onSignInClick = {
viewModel.navigateToSignIn(parentFragmentManager)
if(viewModel.isBrowserLoginEnabled) {
viewModel.signInBrowser(requireActivity())
} else {
viewModel.navigateToSignIn(parentFragmentManager)
}

},
onRegisterClick = {
viewModel.navigateToSignUp(parentFragmentManager)
if (viewModel.isBrowserRegistrationEnabled) {
UrlUtils.openInBrowser(
activity = context,
apiHostUrl = viewModel.apiHostUrl,
url = "/register",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move /register to constants

)
} else {
viewModel.navigateToSignUp(parentFragmentManager)
}
},
onSearchClick = { querySearch ->
viewModel.navigateToDiscovery(parentFragmentManager, querySearch)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,34 @@
package org.openedx.auth.presentation.logistration

import android.app.Activity
import androidx.fragment.app.FragmentManager
import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.launch
import org.openedx.auth.presentation.AuthAnalytics
import org.openedx.auth.presentation.AuthAnalyticsEvent
import org.openedx.auth.presentation.AuthAnalyticsKey
import org.openedx.auth.presentation.AuthRouter
import org.openedx.auth.presentation.sso.BrowserAuthHelper
import org.openedx.core.config.Config
import org.openedx.foundation.extension.takeIfNotEmpty
import org.openedx.foundation.presentation.BaseViewModel
import org.openedx.core.utils.Logger

class LogistrationViewModel(
private val courseId: String,
private val router: AuthRouter,
private val config: Config,
private val analytics: AuthAnalytics,
private val browserAuthHelper: BrowserAuthHelper,
) : BaseViewModel() {

private val logger = Logger("LogistrationViewModel")

private val discoveryTypeWebView get() = config.getDiscoveryConfig().isViewTypeWebView()
val isRegistrationEnabled get() = config.isRegistrationEnabled()
val isBrowserRegistrationEnabled get() = config.isBrowserRegistrationEnabled()
val isBrowserLoginEnabled get() = config.isBrowserLoginEnabled()
val apiHostUrl get() = config.getApiHostURL()

init {
logLogistrationScreenEvent()
Expand All @@ -28,6 +39,16 @@ class LogistrationViewModel(
logEvent(AuthAnalyticsEvent.SIGN_IN_CLICKED)
}

fun signInBrowser(activityContext: Activity) {
viewModelScope.launch {
runCatching {
browserAuthHelper.signIn(activityContext)
}.onFailure {
logger.e { "Browser auth error: $it" }
}
}
}

fun navigateToSignUp(parentFragmentManager: FragmentManager) {
router.navigateToSignUp(parentFragmentManager, courseId, null)
logEvent(AuthAnalyticsEvent.REGISTER_CLICKED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ 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")
Comment on lines +46 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

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
        }
    }

viewModel.signInAuthCode(authCode)
}
LoginScreen(
windowSize = windowSize,
state = state,
Expand All @@ -59,6 +64,10 @@ class SignInFragment : Fragment() {
viewModel.navigateToForgotPassword(parentFragmentManager)
}

AuthEvent.SignInBrowser -> {
viewModel.signInBrowser(requireActivity())
}

AuthEvent.RegisterClick -> {
viewModel.navigateToSignUp(parentFragmentManager)
}
Expand Down Expand Up @@ -107,6 +116,7 @@ internal sealed interface AuthEvent {
data class SignIn(val login: String, val password: String) : AuthEvent
data class SocialSignIn(val authType: AuthType) : AuthEvent
data class OpenLink(val links: Map<String, String>, val link: String) : AuthEvent
object SignInBrowser : AuthEvent
object RegisterClick : AuthEvent
object ForgotPasswordClick : AuthEvent
object BackClick : AuthEvent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ internal data class SignInUIState(
val isGoogleAuthEnabled: Boolean = false,
val isMicrosoftAuthEnabled: Boolean = false,
val isSocialAuthEnabled: Boolean = false,
val isBrowserLoginEnabled: Boolean = false,
val isBrowserRegistrationEnabled: Boolean = false,
val isLogistrationEnabled: Boolean = false,
val isRegistrationEnabled: Boolean = true,
val showProgress: Boolean = false,
val loginSuccess: Boolean = false,
val agreement: RegistrationField? = null,
val loginFailure: Boolean = false,
)
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.openedx.auth.presentation.signin

import android.app.Activity
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentManager
import androidx.lifecycle.LiveData
Expand All @@ -17,6 +18,7 @@ import org.openedx.auth.domain.interactor.AuthInteractor
import org.openedx.auth.domain.model.SocialAuthResponse
import org.openedx.auth.presentation.AgreementProvider
import org.openedx.auth.presentation.AuthAnalytics
import org.openedx.auth.presentation.sso.BrowserAuthHelper
import org.openedx.auth.presentation.AuthAnalyticsEvent
import org.openedx.auth.presentation.AuthAnalyticsKey
import org.openedx.auth.presentation.AuthRouter
Expand Down Expand Up @@ -53,7 +55,8 @@ class SignInViewModel(
private val calendarPreferences: CalendarPreferences,
private val calendarInteractor: CalendarInteractor,
agreementProvider: AgreementProvider,
config: Config,
private val browserAuthHelper: BrowserAuthHelper,
val config: Config,
val courseId: String?,
val infoType: String?,
) : BaseViewModel() {
Expand All @@ -65,6 +68,8 @@ class SignInViewModel(
isFacebookAuthEnabled = config.getFacebookConfig().isEnabled(),
isGoogleAuthEnabled = config.getGoogleConfig().isEnabled(),
isMicrosoftAuthEnabled = config.getMicrosoftConfig().isEnabled(),
isBrowserLoginEnabled = config.isBrowserLoginEnabled(),
isBrowserRegistrationEnabled = config.isBrowserRegistrationEnabled(),
isSocialAuthEnabled = config.isSocialAuthEnabled(),
isLogistrationEnabled = config.isPreLoginExperienceEnabled(),
isRegistrationEnabled = config.isRegistrationEnabled(),
Expand Down Expand Up @@ -158,11 +163,41 @@ class SignInViewModel(
}
}

fun signInBrowser(activityContext: Activity) {
_uiState.update { it.copy(showProgress = true) }
viewModelScope.launch {
runCatching {
browserAuthHelper.signIn(activityContext)
}.onFailure {
logger.e { "Browser auth error: $it" }
}
}
}

fun navigateToSignUp(parentFragmentManager: FragmentManager) {
router.navigateToSignUp(parentFragmentManager, null, null)
logEvent(AuthAnalyticsEvent.REGISTER_CLICKED)
}

fun signInAuthCode(authCode: String) {
_uiState.update { it.copy(showProgress = true) }
viewModelScope.launch {
runCatching {
interactor.loginAuthCode(authCode)
}
.onFailure {
logger.e { "OAuth2 code error: $it" }
onUnknownError()
_uiState.update { it.copy(loginFailure = true) }
}.onSuccess {
logger.d { "Browser login success" }
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove the debug log from here

_uiState.update { it.copy(loginSuccess = true) }
setUserId()
_uiState.update { it.copy(showProgress = false) }
}
}
}

fun navigateToForgotPassword(parentFragmentManager: FragmentManager) {
router.navigateToRestorePassword(parentFragmentManager)
logEvent(AuthAnalyticsEvent.FORGOT_PASSWORD_CLICKED)
Expand Down
Loading
Loading