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
feat: added settings for enabling refresh tokens
refactor: refactors made on comments on pr canonical#278
  • Loading branch information
bencekov committed Dec 6, 2023
1 parent 59d112a commit c43b0be
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 22 deletions.
30 changes: 19 additions & 11 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 @@ -304,8 +304,7 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None:
Args:
event: a :class:`ConfigChangedEvent` to signal that something happened
"""
if self.model.relations[OAUTH]:
self.oauth.update_client_config(client_config=self._oauth_client_config)
self.oauth.update_client_config(client_config=self._oauth_client_config)

self._configure()
self._configure_replication()
Expand Down Expand Up @@ -733,11 +732,17 @@ def _generate_grafana_config(self) -> str:
For now, this only creates database information, since everything else
can be set in ENV variables, but leave for expansion later so we can
hide auth secrets
The feature toggle accessTokenExpirationCheck is also set here. It's needed
for the oauth relation to provide refresh tokens.
"""
configs = []
if self.has_db:
configs.append(self._generate_database_config())

if self.model.relations[OAUTH]:
configs.append(self._generate_oauth_refresh_config())

return "\n".join(configs)

def _generate_database_config(self) -> str:
Expand Down Expand Up @@ -947,6 +952,7 @@ 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",
}
)

Expand Down Expand Up @@ -1427,22 +1433,24 @@ 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()
self._configure()

def _on_oauth_info_removed(self, event: OAuthInfoRemovedEvent) -> None:
"""Event handler for the oauth_info_removed event."""
logger.info("Oauth relation is broken, removing related settings")

# Reset generic_oauth settings
self.restart_grafana()
self._configure()

def _generate_oauth_refresh_config(self) -> str:
"""Generate a configuration for automatic refreshing of oauth authentication.
Returns:
A string containing the required feature toggle information to be stubbed into the config file.
"""
return "[feature_toggles]\naccessTokenExpirationCheck = true\n"


if __name__ == "__main__":
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 c43b0be

Please sign in to comment.