- Use self type annotation instead of extending
- Aggregate OIDC context to a case class
- Use companion object instead of dedicated utility class
This commit is contained in:
Hidetake Iwata
2018-01-25 12:19:47 +09:00
parent ab10b77c50
commit 6db34cbb6b
6 changed files with 34 additions and 43 deletions

View File

@@ -22,6 +22,7 @@ class IndexController extends IndexControllerBase
with IssuesService with IssuesService
with UsersAuthenticator with UsersAuthenticator
with ReferrerAuthenticator with ReferrerAuthenticator
with AccountFederationService
with OpenIDConnectService with OpenIDConnectService
@@ -50,6 +51,7 @@ trait IndexControllerBase extends ControllerBase {
// //
// case class SearchForm(query: String, owner: String, repository: String) // case class SearchForm(query: String, owner: String, repository: String)
case class OidcContext(state: State, nonce: Nonce, redirectBackURI: String)
get("/"){ get("/"){
context.loginAccount.map { account => context.loginAccount.map { account =>
@@ -90,13 +92,11 @@ trait IndexControllerBase extends ControllerBase {
context.settings.oidc.map { oidc => context.settings.oidc.map { oidc =>
val redirectURI = new URI(s"$baseUrl/signin/oidc") val redirectURI = new URI(s"$baseUrl/signin/oidc")
val authenticationRequest = createOIDCAuthenticationRequest(oidc.issuer, oidc.clientID, redirectURI) val authenticationRequest = createOIDCAuthenticationRequest(oidc.issuer, oidc.clientID, redirectURI)
session.setAttribute(Keys.Session.OidcState, authenticationRequest.getState) val redirectBackURI = flash.get(Keys.Flash.Redirect) match {
session.setAttribute(Keys.Session.OidcNonce, authenticationRequest.getNonce)
session.setAttribute(Keys.Session.OidcRedirectBackURI,
flash.get(Keys.Flash.Redirect) match {
case Some(redirectBackURI: String) => redirectBackURI + params.getOrElse("hash", "") case Some(redirectBackURI: String) => redirectBackURI + params.getOrElse("hash", "")
case _ => "/" case _ => "/"
}) }
session.setAttribute(Keys.Session.OidcContext, OidcContext(authenticationRequest.getState, authenticationRequest.getNonce, redirectBackURI))
redirect(authenticationRequest.toURI.toString) redirect(authenticationRequest.toURI.toString)
} getOrElse { } getOrElse {
NotFound() NotFound()
@@ -109,10 +109,10 @@ trait IndexControllerBase extends ControllerBase {
get("/signin/oidc") { get("/signin/oidc") {
context.settings.oidc.map { oidc => context.settings.oidc.map { oidc =>
val redirectURI = new URI(s"$baseUrl/signin/oidc") val redirectURI = new URI(s"$baseUrl/signin/oidc")
Seq(Keys.Session.OidcState, Keys.Session.OidcNonce, Keys.Session.OidcRedirectBackURI).map(session.get(_)) match { session.get(Keys.Session.OidcContext) match {
case Seq(Some(state: State), Some(nonce: Nonce), Some(redirectBackURI: String)) => case Some(context: OidcContext) =>
authenticate(params, redirectURI, state, nonce, oidc) map { account => authenticate(params, redirectURI, context.state, context.nonce, oidc) map { account =>
signin(account, redirectBackURI) signin(account, context.redirectBackURI)
} orElse { } orElse {
flash += "error" -> "Sorry, authentication failed. Please try again." flash += "error" -> "Sorry, authentication failed. Please try again."
session.invalidate() session.invalidate()

View File

@@ -6,7 +6,8 @@ import gitbucket.core.model.{Account, AccountFederation}
import gitbucket.core.util.SyntaxSugars.~ import gitbucket.core.util.SyntaxSugars.~
import org.slf4j.LoggerFactory import org.slf4j.LoggerFactory
trait AccountFederationService extends AccountService { trait AccountFederationService {
self: AccountService =>
private val logger = LoggerFactory.getLogger(classOf[AccountFederationService]) private val logger = LoggerFactory.getLogger(classOf[AccountFederationService])

View File

@@ -2,9 +2,10 @@ package gitbucket.core.service
import java.net.URI import java.net.URI
import com.nimbusds.jose.JOSEException import com.nimbusds.jose.JWSAlgorithm.Family
import com.nimbusds.jose.proc.BadJOSEException import com.nimbusds.jose.proc.BadJOSEException
import com.nimbusds.jose.util.DefaultResourceRetriever import com.nimbusds.jose.util.DefaultResourceRetriever
import com.nimbusds.jose.{JOSEException, JWSAlgorithm}
import com.nimbusds.oauth2.sdk._ import com.nimbusds.oauth2.sdk._
import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic
import com.nimbusds.oauth2.sdk.id.{ClientID, Issuer, State} import com.nimbusds.oauth2.sdk.id.{ClientID, Issuer, State}
@@ -16,12 +17,13 @@ import gitbucket.core.model.Account
import gitbucket.core.model.Profile.profile.blockingApi._ import gitbucket.core.model.Profile.profile.blockingApi._
import org.slf4j.LoggerFactory import org.slf4j.LoggerFactory
import scala.collection.JavaConverters.mapAsJavaMap import scala.collection.JavaConverters.{asScalaSet, mapAsJavaMap}
/** /**
* Service class for the OpenID Connect authentication. * Service class for the OpenID Connect authentication.
*/ */
trait OpenIDConnectService extends AccountFederationService { trait OpenIDConnectService {
self: AccountFederationService =>
private val logger = LoggerFactory.getLogger(classOf[OpenIDConnectService]) private val logger = LoggerFactory.getLogger(classOf[OpenIDConnectService])
@@ -175,3 +177,15 @@ trait OpenIDConnectService extends AccountFederationService {
None None
} }
} }
object OpenIDConnectService {
/**
* All signature algorithms.
*/
val JWS_ALGORITHMS: Map[String, Set[JWSAlgorithm]] = Seq(
"HMAC" -> Family.HMAC_SHA,
"RSA" -> Family.RSA,
"ECDSA" -> Family.EC,
"EdDSA" -> Family.ED
).toMap.map { case (name, family) => (name, asScalaSet(family).toSet) }
}

View File

@@ -28,17 +28,7 @@ object Keys {
/** /**
* Session key for the OpenID Connect authentication. * Session key for the OpenID Connect authentication.
*/ */
val OidcState = "oidc/state" val OidcContext = "oidcContext"
/**
* Session key for the OpenID Connect authentication.
*/
val OidcNonce = "oidc/nonce"
/**
* Session key for the redirect back to after SSO.
*/
val OidcRedirectBackURI = "oidc/redirectBackURI"
/** /**
* Generate session key for the issue search condition. * Generate session key for the issue search condition.

View File

@@ -1,15 +0,0 @@
package gitbucket.core.util
import com.nimbusds.jose.JWSAlgorithm
import com.nimbusds.jose.JWSAlgorithm.Family
import scala.collection.JavaConverters.asScalaSet
object OpenIDConnectUtil {
val JWS_ALGORITHMS: Map[String, Set[JWSAlgorithm]] = Seq(
"HMAC" -> Family.HMAC_SHA,
"RSA" -> Family.RSA,
"ECDSA" -> Family.EC,
"EdDSA" -> Family.ED
).toMap.map { case (name, family) => (name, asScalaSet(family).toSet) }
}

View File

@@ -1,5 +1,6 @@
@(info: Option[Any])(implicit context: gitbucket.core.controller.Context) @(info: Option[Any])(implicit context: gitbucket.core.controller.Context)
@import gitbucket.core.util.{DatabaseConfig, OpenIDConnectUtil} @import gitbucket.core.service.OpenIDConnectService
@import gitbucket.core.util.DatabaseConfig
@gitbucket.core.html.main("System settings"){ @gitbucket.core.html.main("System settings"){
@gitbucket.core.admin.html.menu("system"){ @gitbucket.core.admin.html.menu("system"){
@gitbucket.core.helper.html.information(info) @gitbucket.core.helper.html.information(info)
@@ -321,7 +322,7 @@
<option value="" @if(context.settings.oidc.flatMap(_.jwsAlgorithm) == None){selected}> <option value="" @if(context.settings.oidc.flatMap(_.jwsAlgorithm) == None){selected}>
No signature No signature
</option> </option>
@OpenIDConnectUtil.JWS_ALGORITHMS.map { case (family, algorithms) => @OpenIDConnectService.JWS_ALGORITHMS.map { case (family, algorithms) =>
<optgroup label="@family"> <optgroup label="@family">
@algorithms.map { algorithm => @algorithms.map { algorithm =>
<option value="@algorithm.getName" @if(context.settings.oidc.flatMap(_.jwsAlgorithm) == Some(algorithm)){selected}> <option value="@algorithm.getName" @if(context.settings.oidc.flatMap(_.jwsAlgorithm) == Some(algorithm)){selected}>