Skip to content

Commit

Permalink
fix: fixes based on comments in pr canonical#278
Browse files Browse the repository at this point in the history
  • Loading branch information
bencekov committed Dec 5, 2023
1 parent 719a9a8 commit 5d76e6e
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 19 deletions.
13 changes: 5 additions & 8 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"/usr/local/share/ca-certificates/trusted-ca-cert-$rel_id-ca.crt"
)
OAUTH = "oauth"
OAUTH_SCOPES = "openid email"
OAUTH_SCOPES = "openid email offline_access"
OAUTH_GRANT_TYPES = ["authorization_code", "refresh_token"]


Expand Down Expand Up @@ -312,7 +312,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()

Expand Down Expand Up @@ -947,6 +948,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",
}
)

Expand Down Expand Up @@ -1427,14 +1430,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:
Expand Down
5 changes: 4 additions & 1 deletion tests/integration/oauth_tools/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"KRATOS",
"KRATOS_EXTERNAL_IDP_INTEGRATOR",
"IDENTITY_PLATFORM_LOGIN_UI_OPERATOR",
"SELF_SIGNED_CERTIFICATES",
],
)(
TRAEFIK_ADMIN="traefik-admin",
Expand All @@ -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"])(
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/oauth_tools/dex.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/oauth_tools/oauth_test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 18 additions & 6 deletions tests/integration/test_grafana_oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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__)

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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()
4 changes: 3 additions & 1 deletion tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 5d76e6e

Please sign in to comment.