Opened a new PR for Google Cloud Endpoints OAuth2 support pl kotlinlang #http4k

I don't know much about this, but isn't it just a particular configuration of the existing oauth security? Is there anything to stop us reusing that stuff (with maybe some pre-canned values like we do for OAuthProvider) ?

Ricardo

08/12/2019, 2:10 PM

I didn't use

OAuthSecurity

because Google Cloud Endpoint delegates actual authentication to a proxy. I think

OAuthSecurity

requires a provider, but we don't.

Would you prefer that we amend

OAuthSecurity

and make the provider optional?

Also OAuth is excluded from the

SecurityRenderer

. Not sure if this was deliberate?

dave

08/12/2019, 2:29 PM

The OAuthSecurity definition can take a provider, but it's only through an overloaded method for convienience:

Copy code

data class OAuthSecurity(
    val authorizationUrl: Uri,
    val tokenUrl: Uri,
    val scopes: List<OAuthScope> = emptyList(),
    override val filter: Filter,
    val name: String = "oauthSecurity"
) : Security {
    companion object {
        operator fun invoke(oAuthProvider: OAuthProvider,
                            customScopes: List<OAuthScope>? = null) = OAuthSecurity(
            oAuthProvider.providerConfig.authUri,
            oAuthProvider.providerConfig.tokenUri,
            customScopes ?: oAuthProvider.scopes.map { OAuthScope(it, "") },
            oAuthProvider.authFilter
        )
    }
}

We didn't add OAuth security to the OA2 renderer because we were concentrating on V3.

feel free to retrofit it.

and as an aside, is there any reason that you're not using OA3?

Ricardo

08/12/2019, 3:58 PM

Google Cloud Endpoints doesn't support OpenAPI3 unfortunately, and neither does the ZAP security scanner we use to pen test our API.

I'll update

OAuthSecurity

in that case with the new values as optional and add it to the

SecurityRenderer

Sorry to pester you, but would it possible to take a look at the udpates on this PR? It's something we need for deployment here at JL. Happy to make any changes you folks need.

5 Views