From e20b8790fa15306b73526c58312f4806e6713364 Mon Sep 17 00:00:00 2001 From: Varunalingam Date: Wed, 15 Dec 2021 22:16:08 +0530 Subject: [PATCH 1/2] refactor: make objects internal and move pkce variable out of requests --- DauthSDK/build.gradle | 6 +- .../src/main/java/edu/nitt/delta/DAuth.kt | 70 +++++++++++-------- .../nitt/delta/DauthAccountAuthenticator.kt | 2 + .../java/edu/nitt/delta/helpers/PkceUtils.kt | 2 +- .../nitt/delta/models/AuthorizationRequest.kt | 3 +- .../delta/models/AuthorizationResponse.kt | 4 +- .../edu/nitt/delta/models/TokenRequest.kt | 4 +- .../java/edu/nitt/delta/MainActivity.java | 4 +- .../java/edu/nitt/delta/MainActivity.kt | 2 + 9 files changed, 56 insertions(+), 41 deletions(-) diff --git a/DauthSDK/build.gradle b/DauthSDK/build.gradle index b2744f0..795685f 100644 --- a/DauthSDK/build.gradle +++ b/DauthSDK/build.gradle @@ -18,9 +18,9 @@ android { testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" consumerProguardFiles "consumer-rules.pro" - buildConfigField("String", "CLIENT_ID", "\"${parsedJson.client_id}\"") - buildConfigField("String", "REDIRECT_URI", "\"${parsedJson.redirect_uri}\"") - buildConfigField("String", "CLIENT_SECRET", "\"${parsedJson.client_secret}\"") + buildConfigField("String", "DAUTH_CLIENT_ID", "\"${parsedJson.client_id}\"") + buildConfigField("String", "DAUTH_REDIRECT_URI", "\"${parsedJson.redirect_uri}\"") + buildConfigField("String", "DAUTH_CLIENT_SECRET", "\"${parsedJson.client_secret}\"") } buildTypes { diff --git a/DauthSDK/src/main/java/edu/nitt/delta/DAuth.kt b/DauthSDK/src/main/java/edu/nitt/delta/DAuth.kt index 985b10c..978e0d9 100644 --- a/DauthSDK/src/main/java/edu/nitt/delta/DAuth.kt +++ b/DauthSDK/src/main/java/edu/nitt/delta/DAuth.kt @@ -34,11 +34,10 @@ object DAuth { * clientCreds [ClientCredentials] storing the credentials obtained after client registration in auth.delta.nitt.edu */ private var currentUser: User? = null - private var codeVerifier:String? = null - private val clientCreds: ClientCredentials = ClientCredentials( - BuildConfig.CLIENT_ID, - BuildConfig.REDIRECT_URI, - BuildConfig.CLIENT_SECRET + private val clientCredentials: ClientCredentials = ClientCredentials( + BuildConfig.DAUTH_CLIENT_ID, + BuildConfig.DAUTH_REDIRECT_URI, + BuildConfig.DAUTH_CLIENT_SECRET ) /** @@ -46,16 +45,19 @@ object DAuth { * * @param activity Activity * @param authorizationRequest AuthorizationRequest + * @param isPkceEnabled Boolean which tells whether to choose pkce workflow * @param signInListener ResultListener */ fun signIn( activity: Activity, authorizationRequest: AuthorizationRequest, + isPkceEnabled: Boolean, signInListener: ResultListener ) { signIn( activity, authorizationRequest, + isPkceEnabled, onSuccess = { result -> signInListener.onSuccess(result) }, onFailure = { exception -> signInListener.onFailure(exception) } ) @@ -66,30 +68,31 @@ object DAuth { * * @param activity Activity * @param authorizationRequest AuthorizationRequest + * @param isPkceEnabled Boolean which tells whether to choose pkce workflow * @param onSuccess Lambda Function that is called on successfull login taking Result as member and returns unit * @param onFailure Lambda Function that is called on failure taking Exception as member and returns unit */ fun signIn( activity: Activity, authorizationRequest: AuthorizationRequest, + isPkceEnabled: Boolean, onSuccess: (Result) -> Unit, onFailure: (Exception) -> Unit ) { requestAuthorization( activity, authorizationRequest, + isPkceEnabled, onFailure = { errorState -> onFailure(Exception(errorState.toString())) }, onSuccess = { authorizationResponse -> if (authorizationResponse.state == authorizationRequest.state) { fetchToken( - authorizationRequest, TokenRequest( - client_id = clientCreds.clientId, - client_secret = clientCreds.clientSecret, grant_type = authorizationRequest.grant_type.toString(), code = authorizationResponse.authorizationCode, - redirect_uri = clientCreds.redirectUri + code_verifier = authorizationResponse.codeVerifier ), + isPkceEnabled, onFailure = { e -> onFailure(e) }, onSuccess = { token -> if (authorizationRequest.scopes.contains(Scope.User)) { @@ -97,7 +100,6 @@ object DAuth { token.access_token, onFailure = { e -> onFailure(e) } ) { user -> - currentUser = user if(authorizationRequest.scopes.contains(Scope.OpenID)){ fetchFromJwt( authorizationRequest, @@ -137,16 +139,19 @@ object DAuth { * * @param activity Activity * @param authorizationRequest AuthorizationRequest + * @param isPkceEnabled Boolean which tells whether to choose pkce workflow * @param authorizationListener ResultListener */ fun requestAuthorization( activity: Activity, authorizationRequest: AuthorizationRequest, + isPkceEnabled: Boolean, authorizationListener: ResultListener ) { requestAuthorization( activity, authorizationRequest, + isPkceEnabled, onFailure = { authorizationErrorType -> authorizationListener.onFailure(Exception("$authorizationErrorType")) }, onSuccess = { authorizationResponse -> authorizationListener.onSuccess( @@ -161,12 +166,14 @@ object DAuth { * * @param activity Activity * @param authorizationRequest AuthorizationRequest + * @param isPkceEnabled Boolean which tells whether to choose pkce workflow * @param onFailure Lambda function called on failure taking AuthorizationErrorType as member and returns unit * @param onSuccess Lambda function called on successful authorization taking AuthorizationResponse as member and returns unit */ fun requestAuthorization( activity: Activity, authorizationRequest: AuthorizationRequest, + isPkceEnabled: Boolean, onFailure: (AuthorizationErrorType) -> Unit, onSuccess: (AuthorizationResponse) -> Unit ) { @@ -183,21 +190,19 @@ object DAuth { .scheme(Scheme) .authority(BaseAuthority) .appendPath("authorize") - .appendQueryParameter("client_id", clientCreds.clientId) - .appendQueryParameter("redirect_uri", clientCreds.redirectUri) - .appendQueryParameter( - "response_type", - authorizationRequest.response_type.toString() - ) + .appendQueryParameter("client_id", clientCredentials.clientId) + .appendQueryParameter("redirect_uri", clientCredentials.redirectUri) + .appendQueryParameter("response_type", authorizationRequest.response_type.toString()) .appendQueryParameter("grant_type", authorizationRequest.grant_type.toString()) .appendQueryParameter("state", authorizationRequest.state) .appendQueryParameter("scope", Scope.combineScopes(authorizationRequest.scopes)) .appendQueryParameter("nonce", authorizationRequest.nonce) - if(authorizationRequest.isPkceEnabled){ + var codeVerifier: String? = "" + if(isPkceEnabled){ try { codeVerifier = pkceUtil.generateCodeVerifier() uriBuilder.appendQueryParameter("code_challenge",pkceUtil.generateCodeChallenge( - codeVerifier!!,pkceUtil.getCodeChallengeMethod())) + codeVerifier,pkceUtil.getCodeChallengeMethod())) uriBuilder.appendQueryParameter("code_challenge_method",pkceUtil.getCodeChallengeMethod()) }catch (e: Exception){ onFailure(AuthorizationErrorType.UnableToGenerateCodeVerifier) @@ -211,13 +216,15 @@ object DAuth { onFailure = { onFailure(AuthorizationErrorType.ServerDownError) } ) { url -> val uri: Uri = Uri.parse(url) - if (url.startsWith(clientCreds.redirectUri)) { + if (url.startsWith(clientCredentials.redirectUri)) { if (uri.query.isNullOrBlank() or uri.query.isNullOrEmpty()) { onFailure(AuthorizationErrorType.AuthorizationDenied) } else { val authorizationResponse = AuthorizationResponse( uri.getQueryParameter("code") ?: "", - uri.getQueryParameter("state") ?: "" + uri.getQueryParameter("state") ?: "", + codeVerifier ?: "", + isPkceEnabled ) onSuccess(authorizationResponse) } @@ -243,18 +250,18 @@ object DAuth { /** * Wrapper function to fetch the auth token for java consumers * - * @param authorizationRequest AuthorizationRequest * @param request TokenRequest + * @param isPkceEnabled Boolean which tells whether to choose pkce workflow * @param fetchTokenListener ResultListener */ fun fetchToken( - authorizationRequest: AuthorizationRequest, request: TokenRequest, + isPkceEnabled: Boolean, fetchTokenListener: ResultListener ) { fetchToken( - authorizationRequest, request, + isPkceEnabled, onFailure = { exception -> fetchTokenListener.onFailure(exception) }, onSuccess = { token -> fetchTokenListener.onSuccess(token) } ) @@ -263,21 +270,23 @@ object DAuth { /** * Fetches the auth token * - * @param authorizationRequest AuthorizationRequest * @param request TokenRequest + * @param isPkceEnabled Boolean which tells whether to choose pkce workflow * @param onFailure Lambda function called on failure taking [Exception] as member and returns unit * @param onSuccess Lambda function called after fetching token successfully taking [Token] as member and returns unit */ fun fetchToken( - authorizationRequest: AuthorizationRequest, request: TokenRequest, + isPkceEnabled: Boolean, onFailure: (Exception) -> Unit, onSuccess: (Token) -> Unit ) { var requestAsMap :Map = request.toMap() - if(authorizationRequest.isPkceEnabled) { - requestAsMap = requestAsMap.plus(Pair("code_verifier", codeVerifier!!)) - requestAsMap = requestAsMap.minus("client_secret") + requestAsMap = requestAsMap.plus(Pair("client_id", clientCredentials.clientId)) + requestAsMap = requestAsMap.plus(Pair("redirect_uri", clientCredentials.redirectUri)) + if(!isPkceEnabled){ + requestAsMap = requestAsMap.plus(Pair("client_secret", clientCredentials.clientSecret)) + requestAsMap = requestAsMap.minus("code_verifier") } RetrofitInstance.api.getToken(requestAsMap).enqueue(object : Callback { override fun onResponse(call: Call, response: Response) { @@ -329,7 +338,10 @@ object DAuth { onFailure(Exception(response.code().toString())) return } - response.body()?.let { onSuccess(it) } + response.body()?.let { + currentUser = it + onSuccess(it) + } } override fun onFailure(call: Call, t: Throwable) { diff --git a/DauthSDK/src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt b/DauthSDK/src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt index 75fd045..bb078f3 100644 --- a/DauthSDK/src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt +++ b/DauthSDK/src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt @@ -7,6 +7,7 @@ import android.accounts.AccountManager import android.content.Context import android.content.Intent import android.os.Bundle +import android.util.Log import edu.nitt.delta.api.RetrofitInstance import edu.nitt.delta.constants.ErrorCodeConstants import edu.nitt.delta.constants.ErrorMessageConstants @@ -83,6 +84,7 @@ class DauthAccountAuthenticator(context: Context) : AbstractAccountAuthenticator returnAuthToken(account, response) return Bundle() } else { + Log.d("Hello","Heelo") if (account == null) { response?.onError(ErrorCodeConstants.InternalError, ErrorMessageConstants.NoAccount) return Bundle() diff --git a/DauthSDK/src/main/java/edu/nitt/delta/helpers/PkceUtils.kt b/DauthSDK/src/main/java/edu/nitt/delta/helpers/PkceUtils.kt index 4a1ff48..a49d7be 100644 --- a/DauthSDK/src/main/java/edu/nitt/delta/helpers/PkceUtils.kt +++ b/DauthSDK/src/main/java/edu/nitt/delta/helpers/PkceUtils.kt @@ -5,7 +5,7 @@ import java.security.MessageDigest import java.security.NoSuchAlgorithmException import java.security.SecureRandom -class PkceUtil { +internal class PkceUtil { /** * encodeSettings [encodeSettings] that stores constraints for encoding to string as int variable */ diff --git a/DauthSDK/src/main/java/edu/nitt/delta/models/AuthorizationRequest.kt b/DauthSDK/src/main/java/edu/nitt/delta/models/AuthorizationRequest.kt index e6f7037..100dba1 100644 --- a/DauthSDK/src/main/java/edu/nitt/delta/models/AuthorizationRequest.kt +++ b/DauthSDK/src/main/java/edu/nitt/delta/models/AuthorizationRequest.kt @@ -5,6 +5,5 @@ data class AuthorizationRequest( val grant_type: GrantType, val state: String, val scopes: List, - val nonce: String, - val isPkceEnabled: Boolean + val nonce: String ) diff --git a/DauthSDK/src/main/java/edu/nitt/delta/models/AuthorizationResponse.kt b/DauthSDK/src/main/java/edu/nitt/delta/models/AuthorizationResponse.kt index b1503b1..aea2d7f 100644 --- a/DauthSDK/src/main/java/edu/nitt/delta/models/AuthorizationResponse.kt +++ b/DauthSDK/src/main/java/edu/nitt/delta/models/AuthorizationResponse.kt @@ -2,5 +2,7 @@ package edu.nitt.delta.models data class AuthorizationResponse( val authorizationCode : String, - val state : String + val state : String, + val codeVerifier: String?, + val isPkceEnabled: Boolean ) diff --git a/DauthSDK/src/main/java/edu/nitt/delta/models/TokenRequest.kt b/DauthSDK/src/main/java/edu/nitt/delta/models/TokenRequest.kt index 564ce0e..7920209 100644 --- a/DauthSDK/src/main/java/edu/nitt/delta/models/TokenRequest.kt +++ b/DauthSDK/src/main/java/edu/nitt/delta/models/TokenRequest.kt @@ -1,9 +1,7 @@ package edu.nitt.delta.models data class TokenRequest( - val client_id:String, - val client_secret:String, val grant_type:String, val code: String, - val redirect_uri: String + val code_verifier: String? ) diff --git a/sampleApp/src/Java/java/edu/nitt/delta/MainActivity.java b/sampleApp/src/Java/java/edu/nitt/delta/MainActivity.java index 43046bf..a1672b2 100644 --- a/sampleApp/src/Java/java/edu/nitt/delta/MainActivity.java +++ b/sampleApp/src/Java/java/edu/nitt/delta/MainActivity.java @@ -43,8 +43,8 @@ public void onClick(View view) { GrantType.AuthorizationCode, "1ww12", scopes, - "ncsasd", - true), + "ncsasd"), + true, new ResultListener() { @Override public void onSuccess(@NonNull Result result) { diff --git a/sampleApp/src/Kotlin/java/edu/nitt/delta/MainActivity.kt b/sampleApp/src/Kotlin/java/edu/nitt/delta/MainActivity.kt index 75936b2..5620888 100644 --- a/sampleApp/src/Kotlin/java/edu/nitt/delta/MainActivity.kt +++ b/sampleApp/src/Kotlin/java/edu/nitt/delta/MainActivity.kt @@ -18,6 +18,7 @@ class MainActivity : AppCompatActivity() { super.onCreate(savedInstanceState) setContentView(R.layout.activity_main) val signInButton: DeltaButton = findViewById(R.id.sign_in_button) + signInButton.setOnClickListener { DAuth.signIn( activity = this, @@ -28,6 +29,7 @@ class MainActivity : AppCompatActivity() { listOf(Scope.OpenID,Scope.Profile,Scope.Email,Scope.User), "ncsasd" ), + isPkceEnabled = true, onSuccess = { result: Result -> println("Success: $result") }, From 46efeb3f42d60cc8c7358a7e9994a8278b7092ed Mon Sep 17 00:00:00 2001 From: Varunalingam Date: Sat, 18 Dec 2021 10:07:03 +0530 Subject: [PATCH 2/2] fix: remove logs --- .../src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/DauthSDK/src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt b/DauthSDK/src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt index bb078f3..53cd56e 100644 --- a/DauthSDK/src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt +++ b/DauthSDK/src/main/java/edu/nitt/delta/DauthAccountAuthenticator.kt @@ -84,7 +84,6 @@ class DauthAccountAuthenticator(context: Context) : AbstractAccountAuthenticator returnAuthToken(account, response) return Bundle() } else { - Log.d("Hello","Heelo") if (account == null) { response?.onError(ErrorCodeConstants.InternalError, ErrorMessageConstants.NoAccount) return Bundle()