fix: fixes based on comments in pr #278

src/charm.py

@@ -104,7 +104,7 @@
 PORT = 3000
 
 OAUTH = "oauth"
-OAUTH_SCOPES = "openid email"
+OAUTH_SCOPES = "openid email offline_access"
 OAUTH_GRANT_TYPES = ["authorization_code", "refresh_token"]
 
 
@@ -291,7 +291,8 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None:
 
  def _on_ingress_ready(self, _) -> None:
  """Once Traefik tells us our external URL, make sure we reconfigure Grafana."""
- self.oauth.update_client_config(client_config=self._oauth_client_config)
+ if self.model.relations[OAUTH]:
+ self.oauth.update_client_config(client_config=self._oauth_client_config)
 
  self._configure()
 
@@ -919,6 +920,8 @@ def _build_layer(self) -> Layer:
  "GF_AUTH_GENERIC_OAUTH_AUTH_URL": oauth_provider_info.authorization_endpoint,
  "GF_AUTH_GENERIC_OAUTH_TOKEN_URL": oauth_provider_info.token_endpoint,
  "GF_AUTH_GENERIC_OAUTH_API_URL": oauth_provider_info.userinfo_endpoint,
+ "GF_AUTH_GENERIC_OAUTH_USE_REFRESH_TOKEN": "True",
+ "GF_FEATURE_TOGGLES_ACCESS_TOKEN_EXPIRATION_CHECK": "True",
  }
  )
 
@@ -1357,14 +1360,8 @@ def _oauth_client_config(self) -> ClientConfig:
 
  def _on_oauth_info_changed(self, event: OAuthInfoChangedEvent) -> None:
  """Event handler for the oauth_info_changed event."""
- if not self.unit.is_leader():
- return
-
- self.oauth.update_client_config(client_config=self._oauth_client_config)
  logger.info(f"Received oauth provider info: {self.oauth.get_provider_info()}")
 
- if not event.client_id or not event.client_secret_id:
- return
  self.restart_grafana()
 
  def _on_oauth_info_removed(self, event: OAuthInfoRemovedEvent) -> None:

tests/integration/oauth_tools/constants.py

@@ -23,6 +23,7 @@
  "KRATOS",
  "KRATOS_EXTERNAL_IDP_INTEGRATOR",
  "IDENTITY_PLATFORM_LOGIN_UI_OPERATOR",
+ "SELF_SIGNED_CERTIFICATES",
  ],
 )(
  TRAEFIK_ADMIN="traefik-admin",
@@ -31,14 +32,16 @@
  KRATOS="kratos",
  KRATOS_EXTERNAL_IDP_INTEGRATOR="kratos-external-idp-integrator",
  IDENTITY_PLATFORM_LOGIN_UI_OPERATOR="identity-platform-login-ui-operator",
+ SELF_SIGNED_CERTIFICATES="self-signed-certificates",
 )
 
 OAUTH_RELATION = collections.namedtuple(
- "OAUTH_RELATION", ["OAUTH_APPLICATION", "OAUTH_INTERFACE", "OAUTH_PROXY"]
+ "OAUTH_RELATION", ["OAUTH_APPLICATION", "OAUTH_INTERFACE", "OAUTH_PROXY", "OAUTH_CERTIFICATES"]
 )(
  OAUTH_APPLICATION="hydra",
  OAUTH_INTERFACE="oauth",
  OAUTH_PROXY="traefik-public",
+ OAUTH_CERTIFICATES="self-signed-certificates",
 )
 
 IDENTITY_BUNDLE = collections.namedtuple("IDENTITY_BUNDLE", ["NAME", "CHANNEL"])(

tests/integration/oauth_tools/dex.py

@@ -50,7 +50,7 @@ def update_redirect_uri(self, redirect_uri: str) -> None:
  self._redirect_uri = redirect_uri
  self._apply_dex_resources()
 
- def close(self) -> None:
+ def remove_idp_service(self) -> None:
  # Removes the identity provider deployment
  if self._ops_test.keep_model:
  return

tests/integration/oauth_tools/oauth_test_helper.py

@@ -72,8 +72,8 @@ async def deploy_identity_bundle(ops_test: OpsTest, external_idp_manager: Extern
 
 
 async def access_application_login_page(page: Page, url: str, redirect_login_url: str = ""):
- """Convenience function for navigating the browser to the login page."""
- """
+ """Convenience function for navigating the browser to the login page.
+
  Usage:
  If the url of the application redirects to a login page, pass the application's url as url,
  and a pattern string for the login page as redirect_login_url.

tests/integration/test_grafana_oauth.py

@@ -11,6 +11,7 @@
 import logging
 from helpers import oci_image
 
+import os
 import pytest
 import requests
 from playwright.async_api._generated import Page, BrowserContext
@@ -26,7 +27,7 @@
  verify_page_loads,
  get_cookie_from_browser_by_name,
 )
-from oauth_tools.constants import OAUTH_RELATION
+from oauth_tools.constants import OAUTH_RELATION, EXTERNAL_USER_EMAIL
 
 logger = logging.getLogger(__name__)
 
@@ -62,9 +63,17 @@ async def test_build_and_deploy(ops_test: OpsTest, grafana_charm):
  f"grafana:{OAUTH_RELATION.OAUTH_INTERFACE}", OAUTH_RELATION.OAUTH_APPLICATION
  )
  await ops_test.model.integrate("grafana:ingress", f"{OAUTH_RELATION.OAUTH_PROXY}")
+ await ops_test.model.integrate(
+ "grafana:receive-ca-cert", f"{OAUTH_RELATION.OAUTH_CERTIFICATES}"
+ )
 
  await ops_test.model.wait_for_idle(
- apps=[OAUTH_RELATION.OAUTH_APPLICATION, "grafana", OAUTH_RELATION.OAUTH_PROXY],
+ apps=[
+ OAUTH_RELATION.OAUTH_APPLICATION,
+ "grafana",
+ OAUTH_RELATION.OAUTH_PROXY,
+ OAUTH_RELATION.OAUTH_CERTIFICATES,
+ ],
  status="active",
  raise_on_blocked=False,
  raise_on_error=False,
@@ -80,7 +89,7 @@ async def test_oauth_login_with_identity_bundle(
  grafana_proxy = await get_reverse_proxy_app_url(
  ops_test, OAUTH_RELATION.OAUTH_PROXY, "grafana"
  )
- redirect_login = f"{grafana_proxy}login"
+ redirect_login = os.path.join(grafana_proxy, "login")
 
  await access_application_login_page(
  page=page, url=grafana_proxy, redirect_login_url=redirect_login
@@ -94,7 +103,7 @@ async def test_oauth_login_with_identity_bundle(
  page=page, ops_test=ops_test, external_idp_manager=external_idp_manager
  )
 
- redirect_url = grafana_proxy + "?*"
+ redirect_url = os.path.join(grafana_proxy, "?*")
  await verify_page_loads(page=page, url=redirect_url)
 
  # Verifying that the login flow was successful is application specific.
@@ -103,10 +112,13 @@ async def test_oauth_login_with_identity_bundle(
  browser_context=context, name="grafana_session"
  )
  request = requests.get(
- f"{grafana_proxy}api/user",
+ os.path.join(grafana_proxy, "api/user"),
  headers={"Cookie": f"grafana_session={grafana_session_cookie}"},
  verify=False,
  )
  assert request.status_code == 200
+ assert request.status_code == 200
+ request.raise_for_status()
+ assert request.json()["email"] == EXTERNAL_USER_EMAIL
 
- external_idp_manager.close()
+ external_idp_manager.remove_idp_service()

tests/unit/test_charm.py

@@ -420,7 +420,9 @@ def test_config_is_updated_with_oauth_relation_data(self):
  services["environment"]["GF_AUTH_GENERIC_OAUTH_CLIENT_ID"], "grafana_client_id"
  )
  self.assertEqual(services["environment"]["GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET"], "s3cR#T")
- self.assertEqual(services["environment"]["GF_AUTH_GENERIC_OAUTH_SCOPES"], "openid email")
+ self.assertEqual(
+ services["environment"]["GF_AUTH_GENERIC_OAUTH_SCOPES"], "openid email offline_access"
+ )
  self.assertEqual(
  services["environment"]["GF_AUTH_GENERIC_OAUTH_AUTH_URL"],
  "https://example.oidc.com/oauth2/auth",

tox.ini

@@ -88,6 +88,7 @@ description = Run scenario tests
 [testenv:integration]
 description = Run integration tests
 deps =
+ pytest-asyncio==0.21.1
  aiohttp
  asyncstdlib
  # Libjuju needs to track the juju version