-
Notifications
You must be signed in to change notification settings - Fork 991
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
MBL-1168: PKCE class #1943
Merged
Merged
MBL-1168: PKCE class #1943
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
55dfc02
- CodeVerifier generator class
Arkariang a8cfd3d
- Adding tests for parameters
Arkariang 72e01fb
- Documentation + tests
Arkariang bbc1e24
Merge branch 'master' into imartin/MBL-1168
Arkariang 73bfcdb
- Added PKCE params to Authorization URL
Arkariang f03e907
- linter
Arkariang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
165 changes: 165 additions & 0 deletions
165
app/src/main/java/com/kickstarter/libs/utils/CodeVerifier.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,165 @@ | ||
package com.kickstarter.libs.utils | ||
|
||
import android.util.Base64 | ||
import timber.log.Timber | ||
import java.io.UnsupportedEncodingException | ||
import java.security.MessageDigest | ||
import java.security.NoSuchAlgorithmException | ||
import java.security.SecureRandom | ||
import java.util.regex.Pattern | ||
|
||
/** | ||
* Generates code verifiers and challenges for PKCE exchange. | ||
* | ||
* @see [Proof Key for Code Exchange by OAuth Public Clients](https://datatracker.ietf.org/doc/html/rfc7636) | ||
*/ | ||
class CodeVerifier { | ||
companion object { | ||
/** | ||
* The minimum permitted length for a code verifier. | ||
* | ||
* @see "Proof Key for Code Exchange by OAuth Public Clients" | ||
*/ | ||
const val MIN_CODE_VERIFIER_LENGTH = 43 | ||
|
||
/** | ||
* The maximum permitted length for a code verifier. | ||
* | ||
* @see "Proof Key for Code Exchange by OAuth Public Clients" | ||
*/ | ||
const val MAX_CODE_VERIFIER_LENGTH = 128 | ||
|
||
/** | ||
* The default entropy (in bytes) used for the code verifier. | ||
*/ | ||
const val DEFAULT_CODE_VERIFIER_ENTROPY = 64 | ||
|
||
/** | ||
* The minimum permitted entropy (in bytes) for use with | ||
* [.generateRandomCodeVerifier]. | ||
*/ | ||
const val MIN_CODE_VERIFIER_ENTROPY = 32 | ||
|
||
/** | ||
* The maximum permitted entropy (in bytes) for use with | ||
* [.generateRandomCodeVerifier]. | ||
*/ | ||
const val MAX_CODE_VERIFIER_ENTROPY = 96 | ||
|
||
/** | ||
* Base64 encoding settings used for generated code verifiers. | ||
*/ | ||
private const val PKCE_BASE64_ENCODE_SETTINGS: Int = | ||
Base64.NO_WRAP or Base64.NO_PADDING or Base64.URL_SAFE | ||
|
||
/** | ||
* Regex for legal code verifier strings, as defined in the spec. | ||
* | ||
* @see "Proof Key for Code Exchange by OAuth Public Clients" | ||
*/ | ||
private val REGEX_CODE_VERIFIER: Pattern = | ||
Pattern.compile("^[0-9a-zA-Z\\-._~]{43,128}$") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll steal this for iOS, nicer than what I was using. |
||
|
||
/** | ||
* SHA-256 based code verifier challenge method. | ||
* | ||
* @see "Proof Key for Code Exchange by OAuth Public Clients" | ||
*/ | ||
const val CODE_CHALLENGE_METHOD_S256 = "S256" | ||
|
||
/** | ||
* Plain-text code verifier challenge method. This is only used by AppAuth for Android if | ||
* SHA-256 is not supported on this platform. | ||
* | ||
* @see "Proof Key for Code Exchange by OAuth Public Clients" | ||
*/ | ||
const val CODE_CHALLENGE_METHOD_PLAIN = "plain" | ||
|
||
const val ERROR_TOO_SHORT = "codeVerifier length is shorter than allowed by the PKCE specification" | ||
|
||
const val ERROR_TOO_LONG = "codeVerifier length is longer than allowed by the PKCE specification" | ||
|
||
const val ERROR_DO_NOT_MATCH = "codeVerifier string does not match legal code verifier strings REGEX" | ||
|
||
/** | ||
* Throws an IllegalArgumentException if the provided code verifier is invalid. | ||
* | ||
* @see [4.1. Client Creates a Code Verifier](https://datatracker.ietf.org/doc/html/rfc7636#section-4.1) | ||
*/ | ||
fun checkCodeVerifier(codeVerifier: String) { | ||
require( | ||
MIN_CODE_VERIFIER_LENGTH <= codeVerifier.length | ||
) { ERROR_TOO_SHORT } | ||
require( | ||
codeVerifier.length <= MAX_CODE_VERIFIER_LENGTH | ||
) { ERROR_TOO_LONG } | ||
require( | ||
REGEX_CODE_VERIFIER.matcher(codeVerifier).matches() | ||
) { ERROR_DO_NOT_MATCH } | ||
} | ||
|
||
/** | ||
* Generates a random code verifier string using the provided entropy source and the specified | ||
* number of bytes of entropy. | ||
*/ | ||
/** | ||
* Generates a random code verifier string using [SecureRandom] as the source of | ||
* entropy, with the default entropy quantity as defined by | ||
* [.DEFAULT_CODE_VERIFIER_ENTROPY]. | ||
* | ||
* @see [Client Creates a Code Verifier](https://datatracker.ietf.org/doc/html/rfc7636#section-4.1) | ||
*/ | ||
fun generateRandomCodeVerifier( | ||
entropySource: SecureRandom = SecureRandom(), | ||
entropyBytes: Int = DEFAULT_CODE_VERIFIER_ENTROPY | ||
): String { | ||
require( | ||
MIN_CODE_VERIFIER_ENTROPY <= entropyBytes | ||
) { "entropyBytes is less than the minimum permitted" } | ||
require( | ||
entropyBytes <= MAX_CODE_VERIFIER_ENTROPY | ||
) { "entropyBytes is greater than the maximum permitted" } | ||
|
||
val randomBytes = ByteArray(entropyBytes) | ||
entropySource.nextBytes(randomBytes) | ||
return Base64.encodeToString(randomBytes, PKCE_BASE64_ENCODE_SETTINGS) | ||
} | ||
|
||
/** | ||
* Produces a challenge from a code verifier, using SHA-256 as the challenge method if the | ||
* system supports it (all Android devices _should_ support SHA-256), and falls back | ||
* to the ["plain" challenge type][CODE_CHALLENGE_METHOD_PLAIN] if | ||
* unavailable. | ||
* | ||
* See [Example for the S256 code_challenge_method](https://datatracker.ietf.org/doc/html/rfc7636#appendix-B) | ||
*/ | ||
fun generateCodeChallenge(codeVerifier: String): String { | ||
return try { | ||
val sha256Digester = MessageDigest.getInstance("SHA-256") | ||
sha256Digester.update(codeVerifier.toByteArray(charset("ISO_8859_1"))) | ||
val digestBytes = sha256Digester.digest() | ||
Base64.encodeToString(digestBytes, PKCE_BASE64_ENCODE_SETTINGS) | ||
} catch (e: NoSuchAlgorithmException) { | ||
Timber.w("SHA-256 is not supported on this device! Using plain challenge", e) | ||
codeVerifier | ||
} catch (e: UnsupportedEncodingException) { | ||
Timber.e("ISO-8859-1 encoding not supported on this device!", e) | ||
throw IllegalStateException("ISO-8859-1 encoding not supported", e) | ||
} | ||
} | ||
|
||
private val codeVerifierChallengeMethod: String | ||
/** | ||
* Returns the challenge method utilized on this system: typically | ||
* [SHA-256][CODE_CHALLENGE_METHOD_S256] if supported by | ||
* the system, [plain][CODE_CHALLENGE_METHOD_PLAIN] otherwise. | ||
*/ | ||
get() = try { | ||
MessageDigest.getInstance("SHA-256") | ||
// no exception, so SHA-256 is supported | ||
CODE_CHALLENGE_METHOD_S256 | ||
} catch (e: NoSuchAlgorithmException) { | ||
CODE_CHALLENGE_METHOD_PLAIN | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
129 changes: 129 additions & 0 deletions
129
app/src/test/java/com/kickstarter/libs/utils/CodeVerifierTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
package com.kickstarter.libs.utils | ||
|
||
import com.kickstarter.KSRobolectricTestCase | ||
import com.kickstarter.libs.utils.CodeVerifier.Companion.ERROR_DO_NOT_MATCH | ||
import com.kickstarter.libs.utils.CodeVerifier.Companion.ERROR_TOO_LONG | ||
import com.kickstarter.libs.utils.CodeVerifier.Companion.ERROR_TOO_SHORT | ||
import com.kickstarter.libs.utils.CodeVerifier.Companion.MAX_CODE_VERIFIER_ENTROPY | ||
import com.kickstarter.libs.utils.CodeVerifier.Companion.MIN_CODE_VERIFIER_ENTROPY | ||
import com.kickstarter.libs.utils.CodeVerifier.Companion.generateCodeChallenge | ||
import com.kickstarter.libs.utils.CodeVerifier.Companion.generateRandomCodeVerifier | ||
import org.junit.Assert.assertThrows | ||
import org.junit.Test | ||
|
||
class CodeVerifierTest : KSRobolectricTestCase() { | ||
|
||
@Test | ||
fun checkCodeVerifier_tooShort_throwsException() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are great. With the exception of entropy, I can port the exact same checks to iOS as well. |
||
val codeVerifier = createString(CodeVerifier.MIN_CODE_VERIFIER_LENGTH - 1) | ||
val exception = assertThrows(IllegalArgumentException::class.java) { | ||
CodeVerifier.checkCodeVerifier(codeVerifier) | ||
} | ||
|
||
assertEquals(exception.message, ERROR_TOO_SHORT) | ||
} | ||
|
||
@Test | ||
fun checkCodeVerifier_tooLong_throwsException() { | ||
val codeVerifier = createString(CodeVerifier.MAX_CODE_VERIFIER_LENGTH + 1) | ||
val exception = assertThrows(IllegalArgumentException::class.java) { | ||
CodeVerifier.checkCodeVerifier(codeVerifier) | ||
} | ||
|
||
assertEquals(exception.message, ERROR_TOO_LONG) | ||
} | ||
|
||
@Test | ||
fun checkCodeVerifier_languageSentence_notValid() { | ||
val sentence = "Hello, world. I am a string. Hello, world. I am a string." | ||
val exception = assertThrows(IllegalArgumentException::class.java) { | ||
CodeVerifier.checkCodeVerifier(sentence) | ||
} | ||
|
||
assertEquals(exception.message, ERROR_DO_NOT_MATCH) | ||
} | ||
|
||
@Test | ||
fun generateRandomCodeVerifier_tooLittleEntropy_throwsException() { | ||
val exception = assertThrows(IllegalArgumentException::class.java) { | ||
CodeVerifier.generateRandomCodeVerifier( | ||
entropyBytes = MIN_CODE_VERIFIER_ENTROPY - 1 | ||
) | ||
} | ||
assertEquals(exception.message, "entropyBytes is less than the minimum permitted") | ||
} | ||
|
||
@Test | ||
fun generateRandomCodeVerifier_tooMuchEntropy_throwsException() { | ||
val exception = assertThrows(IllegalArgumentException::class.java) { | ||
generateRandomCodeVerifier( | ||
entropyBytes = MAX_CODE_VERIFIER_ENTROPY + 1 | ||
) | ||
} | ||
assertEquals(exception.message, "entropyBytes is greater than the maximum permitted") | ||
} | ||
|
||
/** | ||
* Generates random String with @param length | ||
*/ | ||
private fun createString(length: Int): String { | ||
val strChars = CharArray(length) | ||
for (i in strChars.indices) { | ||
strChars[i] = 'a' | ||
} | ||
return String(strChars) | ||
} | ||
|
||
@Test | ||
fun givenSentence_generateCodeChallengeWithSHA256Hash() { | ||
// - Use https://oauth.school/exercise/refresh/ to obtain givenCodeChallenge | ||
val givenCodeChallenge = "wcaGQDnzgCSNMKc1Jcg1FCfH-0aNWLexAF8-NyegQqE" | ||
|
||
val givenCodeVerifier = "Hello, world. I am a string." | ||
val generatedChallenge = generateCodeChallenge(givenCodeVerifier) | ||
assertEquals(givenCodeChallenge, generatedChallenge) | ||
} | ||
|
||
@Test | ||
fun givenCodeVerifierMinEntropy_generateCodeChallengeWithSHA256Hash() { | ||
// - [givenVerifier] generated using generateRandomCodeVerifier(MIN_CODE_VERIFIER_ENTROPY) | ||
// - Use https://oauth.school/exercise/refresh/ to obtain [givenCodeChallenge] | ||
val givenVerifier = "HaTkldnGaT3PcENU5EAY8rtDDNIikQSvBXFFEYBa3MA" | ||
val codeChallenge = generateCodeChallenge(givenVerifier) | ||
|
||
val givenCodeChallenge = "khL4OfhvX-uphctb0gMMmE_O5xNX-MfjMPvHxAbpsZk" | ||
assertEquals(codeChallenge, givenCodeChallenge) | ||
} | ||
|
||
@Test | ||
fun givenCodeVerifierDefaultEntropy_generateCodeChallengeWithSHA256Hash() { | ||
// - [givenVerifier] generated using generateRandomCodeVerifier(DEFAULT_CODE_VERIFIER_ENTROPY) | ||
// - Use https://oauth.school/exercise/refresh/ to obtain [givenCodeChallenge] | ||
val givenVerifier = "BAxigyqguFpLKXnGiqc0iabt-Epr3YL-wJvPL0CfDSTGB45_jOwrSrFa0_T4FK5y9amhhYQAk-Bkr2zpD8Gpxw" | ||
val codeChallenge = generateCodeChallenge(givenVerifier) | ||
|
||
val givenCodeChallenge = "DcimCRjKEAmp3cl0mFMc12oCsHfN931jzpot2HCkBNo" | ||
assertEquals(codeChallenge, givenCodeChallenge) | ||
} | ||
@Test | ||
fun givenCodeVerifierMaxEntropy_generateCodeChallengeWithSHA256Hash() { | ||
// - [givenVerifier] generated using generateRandomCodeVerifier(MAX_CODE_VERIFIER_ENTROPY) | ||
// - Use https://oauth.school/exercise/refresh/ to obtain [givenCodeChallenge] | ||
val givenVerifier = "YfZIzxXTx7Dc58fLNl2uO6cRzWSevpEPeKSXGBFhN8fisOA3XjV_AF0Buz2ZjYxu7S30j15dlzPCzbtHZEHCqAo94YcaZV4JNfJYWCi1jWavu8UUSdCw9n6Y3dinTRfe" | ||
val codeChallenge = generateCodeChallenge(givenVerifier) | ||
|
||
val givenCodeChallenge = "cJXeRcpWhJLlsCD8DG3OLkLtjGF8yip6Hf0Jd560Pgg" | ||
assertEquals(codeChallenge, givenCodeChallenge) | ||
} | ||
|
||
@Test | ||
fun generateSeveralCodeVerifiers_checkDoNotMatch() { | ||
val codeVerifierA = generateRandomCodeVerifier() | ||
val codeVerifierB = generateRandomCodeVerifier() | ||
val codeVerifierD = generateRandomCodeVerifier() | ||
|
||
assertTrue(codeVerifierA != codeVerifierB) | ||
assertTrue(codeVerifierA != codeVerifierD) | ||
assertTrue(codeVerifierB != codeVerifierD) | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I don't believe these checks exist in the iOS version of the AppAuth library (at least, I didn't see them, and don't see anything about them when I search.) Is needing to check the entropy level an Android-specific problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is OS related, in the sense that the entropy level adds more or less "randomness". It does impact the performance though, higher levels require more computer power.
In our case for now I'll specify MAX_CODE_VERIFIER_ENTROPY as the difference in computing time is minimal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it may just be that iOS doesn't allow you to specify the entropy, at least in the simple interface for crypto I'm using.