From be3ef0e209f190f640c8bde0069c0215bd7380fd Mon Sep 17 00:00:00 2001 From: Benjamin Cavy Date: Mon, 10 Jun 2024 20:42:23 +0200 Subject: [PATCH] feat: improve OIDC * make scopes configurable * allow to choose which claim will be used as username / email for users Fix #827 Fix #826 --- .gitignore | 1 + .../datastores/ConfigurationDatastore.scala | 15 +- .../izanami/models/IzanamiConfiguration.scala | 5 +- app/fr/maif/izanami/web/LoginController.scala | 52 ++- conf/application.conf | 6 + conf/dev.conf | 3 + izanami-frontend/src/App.tsx | 18 +- izanami-frontend/src/pages/login.tsx | 12 +- izanami-frontend/src/pages/profile.tsx | 5 +- .../default/generated.js | 10 +- manual/.docusaurus/client-modules.js | 8 +- manual/.docusaurus/registry.js | 164 +++++---- manual/.docusaurus/routes.js | 43 ++- manual/.docusaurus/routesChunkNames.json | 341 ++++++++++-------- manual/.docusaurus/site-metadata.json | 4 +- manual/docs/04-guides/11-configuration.mdx | 21 +- test/fr/maif/izanami/api/LoginAPISpec.scala | 10 +- 17 files changed, 428 insertions(+), 290 deletions(-) diff --git a/.gitignore b/.gitignore index 3a33bd8b1..3d39d1605 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,4 @@ project/.bloop/ project/metals.sbt project/project/ izanami-frontend/playwright/.auth/ +.oidc-secrets \ No newline at end of file diff --git a/app/fr/maif/izanami/datastores/ConfigurationDatastore.scala b/app/fr/maif/izanami/datastores/ConfigurationDatastore.scala index 2684a19aa..7ed73b5c0 100644 --- a/app/fr/maif/izanami/datastores/ConfigurationDatastore.scala +++ b/app/fr/maif/izanami/datastores/ConfigurationDatastore.scala @@ -40,8 +40,19 @@ class ConfigurationDatastore(val env: Env) extends Datastore { clientSecret <- env.configuration.getOptional[String]("app.openid.client-secret"); authorizeUrl <- env.configuration.getOptional[String]("app.openid.authorize-url"); tokenUrl <- env.configuration.getOptional[String]("app.openid.token-url"); - redirectUrl <- env.configuration.getOptional[String]("app.openid.redirect-url") - ) yield OIDCConfiguration(clientId=clientId, clientSecret=clientSecret, authorizeUrl=authorizeUrl, tokenUrl=tokenUrl, redirectUrl=redirectUrl) + redirectUrl <- env.configuration.getOptional[String]("app.openid.redirect-url"); + usernameField <- env.configuration.getOptional[String]("app.openid.username-field"); + emailField <- env.configuration.getOptional[String]("app.openid.email-field"); + scopes <- env.configuration.getOptional[String]("app.openid.scopes") + ) yield OIDCConfiguration(clientId=clientId, + clientSecret=clientSecret, + authorizeUrl=authorizeUrl, + tokenUrl=tokenUrl, + redirectUrl=redirectUrl, + usernameField = usernameField, + emailField = emailField, + scopes = scopes.split(" ").toSet + ) } def readConfiguration(): Future[Either[IzanamiError, IzanamiConfiguration]] = { diff --git a/app/fr/maif/izanami/models/IzanamiConfiguration.scala b/app/fr/maif/izanami/models/IzanamiConfiguration.scala index eebb5c040..bbddc36a5 100644 --- a/app/fr/maif/izanami/models/IzanamiConfiguration.scala +++ b/app/fr/maif/izanami/models/IzanamiConfiguration.scala @@ -18,7 +18,10 @@ case class OIDCConfiguration( clientSecret: String, authorizeUrl: String, tokenUrl: String, - redirectUrl: String + redirectUrl: String, + usernameField: String, + emailField: String, + scopes: Set[String] ) case class IzanamiConfiguration( diff --git a/app/fr/maif/izanami/web/LoginController.scala b/app/fr/maif/izanami/web/LoginController.scala index 330acf653..8e3e1196d 100644 --- a/app/fr/maif/izanami/web/LoginController.scala +++ b/app/fr/maif/izanami/web/LoginController.scala @@ -6,7 +6,7 @@ import fr.maif.izanami.models.User.userRightsWrites import fr.maif.izanami.models.{OIDC, OIDCConfiguration, Rights, User} import fr.maif.izanami.utils.syntax.implicits.BetterSyntax import pdi.jwt.{JwtJson, JwtOptions} -import play.api.libs.json.Json +import play.api.libs.json.{JsObject, Json} import play.api.libs.ws.WSAuthScheme import play.api.mvc.Cookie.SameSite import play.api.mvc._ @@ -22,46 +22,58 @@ class LoginController( implicit val ec: ExecutionContext = env.executionContext; def openIdConnect = Action { - env.datastores.configuration.readOIDCConfiguration() match { - case None => MissingOIDCConfigurationError().toHttpResponse - case Some(OIDCConfiguration(clientId, _, authorizeUrl, _, redirectUrl)) => Redirect(s"${authorizeUrl}?scope=openid%20profile%20email%20name&client_id=${clientId}&response_type=code&redirect_uri=${redirectUrl}") + env.datastores.configuration.readOIDCConfiguration() match { + case None => MissingOIDCConfigurationError().toHttpResponse + case Some(OIDCConfiguration(clientId, _, authorizeUrl, _, redirectUrl, _, _, scopes)) => { + val hasOpenIdInScope = scopes.exists(s => s.equalsIgnoreCase("openid")) + val actualScope = (if (!hasOpenIdInScope) scopes + "openid" else scopes).mkString("%20") + Redirect( + s"${authorizeUrl}?scope=$actualScope&client_id=${clientId}&response_type=code&redirect_uri=${redirectUrl}" + ) } + } } def openIdCodeReturn = Action.async { implicit request => // TODO handle refresh_token { for ( - code <- request.body.asJson.flatMap(json => (json \ "code").get.asOpt[String]); - OIDCConfiguration(clientId, clientSecret, _, tokenUrl, redirectUrl) <- env.datastores.configuration.readOIDCConfiguration() + code <- request.body.asJson.flatMap(json => (json \ "code").get.asOpt[String]); + OIDCConfiguration(clientId, clientSecret, _, tokenUrl, redirectUrl, usernameField, emailField, _) <- + env.datastores.configuration.readOIDCConfiguration() ) yield env.Ws .url(tokenUrl) .withAuth(clientId, clientSecret, WSAuthScheme.BASIC) .withHttpHeaders(("content-type", "application/x-www-form-urlencoded")) .post(Map("grant_type" -> "authorization_code", "code" -> code, "redirect_uri" -> redirectUrl)) - //.post(s"grant_type=authorization_code&code=${code}&redirect_uri=${redirectUrl}") .flatMap(r => { val maybeToken = (r.json \ "id_token").get.asOpt[String] maybeToken.fold(Future(InternalServerError(Json.obj("message" -> "Failed to retrieve token"))))(token => { val maybeClaims = JwtJson.decode(token, JwtOptions(signature = false)) maybeClaims.toOption - .flatMap(claims => claims.subject) - .map(userId => - env.datastores.users - .findUser(userId) - .flatMap(maybeUser => - maybeUser.fold( - // TODO handle mail - env.datastores.users.createUser(User(userId, userType = OIDC).withRights(Rights.EMPTY)) - )(user => Future(Right(user.withRights(Rights.EMPTY)))).map(either => either.map(_ => userId)) - ) - ) + .flatMap(claims => Json.parse(claims.content).asOpt[JsObject]) + .flatMap(json => { + for ( + username <- (json \ usernameField).asOpt[String]; + email <- (json \ emailField).asOpt[String] + ) + yield env.datastores.users + .findUser(username) + .flatMap(maybeUser => + maybeUser + .fold( + env.datastores.users + .createUser(User(username, email = email, userType = OIDC).withRights(Rights.EMPTY)) + )(user => Future(Right(user.withRights(Rights.EMPTY)))) + .map(either => either.map(_ => username)) + ) + }) .getOrElse(Future(Left(InternalServerError(Json.obj("message" -> "Failed to read token claims"))))) .flatMap { // TODO refactor this whole method case Right(username) => env.datastores.users.createSession(username).map(id => Right(id)) - case Left(err) => Future(Left(err)) + case Left(err) => Future(Left(err)) } .map(maybeId => { maybeId @@ -133,7 +145,7 @@ class LoginController( ) ) } - case _ => Future(Unauthorized(Json.obj("message" -> "Missing credentials"))) + case _ => Future(Unauthorized(Json.obj("message" -> "Missing credentials"))) } } } diff --git a/conf/application.conf b/conf/application.conf index 767f05438..4d46e2c21 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -32,6 +32,12 @@ app { token-url = ${?IZANAMI_OPENID_TOKEN_URL} redirect-url = ${?app.exposition.url}"/login" redirect-url = ${?IZANAMI_OPENID_REDIRECT_URL} + scopes = "openid email profile" + scopes = ${?IZANAMI_OPENID_SCOPES} + email-field = "email" + email-field = ${?IZANAMI_OPENID_EMAIL_FIELD} + username-field = "name" + username-field = ${?IZANAMI_OPENID_USERNAME_FIELD} } wasmo { url = ${?IZANAMI_WASMO_URL} diff --git a/conf/dev.conf b/conf/dev.conf index 2367bfcff..8d45018c9 100644 --- a/conf/dev.conf +++ b/conf/dev.conf @@ -18,6 +18,9 @@ app { authorize-url = "http://localhost:9001/auth" token-url = "http://localhost:9001/token" redirect-url = "http://localhost:3000/login" + scopes = "email profile" + username-field = name + email-field = email } admin { password = "ADMIN_DEFAULT_PASSWORD" diff --git a/izanami-frontend/src/App.tsx b/izanami-frontend/src/App.tsx index 17963c80c..efa0d6e26 100644 --- a/izanami-frontend/src/App.tsx +++ b/izanami-frontend/src/App.tsx @@ -416,8 +416,7 @@ function RedirectToFirstTenant(): JSX.Element { } function Layout() { - const { user, setUser, logout, expositionUrl, setExpositionUrl } = - useContext(IzanamiContext); + const { user, setUser, logout, expositionUrl } = useContext(IzanamiContext); const loading = !user?.username || !expositionUrl; useEffect(() => { if (!user?.username) { @@ -426,11 +425,6 @@ function Layout() { .then((user) => setUser(user)) .catch(console.error); } - if (!expositionUrl) { - fetch("/api/admin/exposition") - .then((response) => response.json()) - .then(({ url }) => setExpositionUrl(url)); - } }, [user?.username]); if (loading) { @@ -654,12 +648,22 @@ export class App extends Component { } } + fetchExpositionUrlIfNeeded(): void { + if (!this.state.expositionUrl) { + fetch("/api/admin/exposition") + .then((response) => response.json()) + .then(({ url }) => this.setState({ expositionUrl: url })); + } + } + componentDidMount(): void { this.fetchIntegrationsIfNeeded(); + this.fetchExpositionUrlIfNeeded(); } componentDidUpdate(): void { this.fetchIntegrationsIfNeeded(); + this.fetchExpositionUrlIfNeeded; setupLightMode(); } diff --git a/izanami-frontend/src/pages/login.tsx b/izanami-frontend/src/pages/login.tsx index 2c8bf1961..84c20a829 100644 --- a/izanami-frontend/src/pages/login.tsx +++ b/izanami-frontend/src/pages/login.tsx @@ -6,6 +6,7 @@ import Logo from "../../izanami.png"; import { Configuration, TUser } from "../utils/types"; import { useMutation } from "react-query"; import { updateConfiguration } from "../utils/queries"; +import { Loader } from "../components/Loader"; export function Login(props: any) { const code = new URLSearchParams(props.location.search).get("code"); @@ -42,7 +43,11 @@ function TokenWaitScreen({ code }: { code: string }) { if (error) { return
{error}
; } else if (fetching) { - return
Fetching...
; + return ( +
+ +
+ ); } else { return
Fetched !!!
; } @@ -51,7 +56,7 @@ function TokenWaitScreen({ code }: { code: string }) { function LoginForm(props: { req?: string }) { const navigate = useNavigate(); const req = props.req; - const { setUser, integrations } = useContext(IzanamiContext); + const { setUser, integrations, expositionUrl } = useContext(IzanamiContext); const [error, setError] = useState(""); const [loading, setLoading] = useState(false); @@ -164,8 +169,7 @@ function LoginForm(props: { req?: string }) {