Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the section name in wait_for_oidc_server function #3415

Merged
merged 5 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/pbench/cli/server/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,16 @@ def run_gunicorn(server_config: PbenchServerConfig, logger: Logger) -> int:
except OpenIDClient.NotConfigured as exc:
logger.warning("OpenID Connect client not configured, {}", exc)
notifier.notify("STOPPING=1")
notifier.notify("STATUS=OPENID broker not configured")
return 1
except (
OpenIDClient.ServerConnectionError,
OpenIDClient.ServerConnectionTimedOut,
) as exc:
logger.warning("OpenID Connect client not reachable, {}", exc)
notifier.notify("STOPPING=1")
notifier.notify("STATUS=OPENID broker not responding")
return 1
else:
logger.info("Pbench server using OIDC server {}", oidc_server)

Expand Down
24 changes: 10 additions & 14 deletions lib/pbench/server/api/resources/endpoint_configure.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from configparser import NoOptionError, NoSectionError
from http import HTTPStatus
import re
from typing import Any, Dict
Expand Down Expand Up @@ -50,7 +49,7 @@ def get(self):
Return server configuration information required by web clients
including the Pbench dashboard UI. This includes:

openid-connect: A JSON object containing the OpenID Connect parameters
openid: A JSON object containing the OpenID Connect parameters
dbutenhof marked this conversation as resolved.
Show resolved Hide resolved
required for the web client to use OIDC authentication.
identification: The Pbench server name and version
uri: A dict of server API templates, where each template defines a
Expand Down Expand Up @@ -150,18 +149,15 @@ def get(self):
"uri": templates,
}

try:
client = self.server_config.get("openid", "client")
realm = self.server_config.get("openid", "realm")
server = self.server_config.get("openid", "server_url")
except (NoOptionError, NoSectionError):
pass
else:
endpoints["openid"] = {
"client": client,
"realm": realm,
"server": server,
}
client = self.server_config.get("openid", "client")
realm = self.server_config.get("openid", "realm")
server = self.server_config.get("openid", "server_url")

endpoints["openid"] = {
"client": client,
"realm": realm,
"server": server,
}

try:
response = jsonify(endpoints)
Expand Down
2 changes: 1 addition & 1 deletion lib/pbench/server/auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def wait_for_oidc_server(
times out.
"""
try:
oidc_server = server_config.get("openid-connect", "server_url")
oidc_server = server_config.get("openid", "server_url")
except (NoOptionError, NoSectionError) as exc:
raise OpenIDClient.NotConfigured() from exc

Expand Down
59 changes: 59 additions & 0 deletions lib/pbench/test/unit/server/auth/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import pytest
import requests
from requests.structures import CaseInsensitiveDict
import responses

from pbench.server import JSON
import pbench.server.auth
Expand Down Expand Up @@ -289,6 +290,64 @@ def get(self, path: str, **kwargs) -> MockResponse:
class TestOpenIDClient:
"""Verify the OpenIDClient class."""

@responses.activate
def test_wait_for_oidc_server_fail(self, make_logger):
"""Verify .wait_for_oidc_server() failure mode"""
# Start with an empty configuration, no openid section
config = configparser.ConfigParser()
with pytest.raises(OpenIDClient.NotConfigured):
OpenIDClient.wait_for_oidc_server(config, make_logger)
# Missing "server_url"
section = {}
config["openid"] = section
with pytest.raises(OpenIDClient.NotConfigured):
OpenIDClient.wait_for_oidc_server(config, make_logger)

section = {"server_url": "https://example.com"}
config["openid"] = section

# Keycloak health returning response but status is not UP
responses.add(
responses.GET,
"https://example.com/health",
body='{"status": "DOWN","checks": []}',
content_type="application/json",
)

with pytest.raises(OpenIDClient.ServerConnectionTimedOut):
OpenIDClient.wait_for_oidc_server(config, make_logger)

# Keycloak health returning network exception and no content
responses.add(
responses.GET,
"https://example.com/health",
body=Exception("some network exception"),
)

with pytest.raises(OpenIDClient.ServerConnectionError):
OpenIDClient.wait_for_oidc_server(config, make_logger)

@responses.activate
def test_wait_for_oidc_server_succ(self, make_logger):
"""Verify .wait_for_oidc_server() success mode"""

config = configparser.ConfigParser()
section = {"server_url": "https://example.com"}
config["openid"] = section

# Keycloak health returning response with status UP
responses.add(
responses.GET,
"https://example.com/health",
body='{"status": "UP","checks": []}',
content_type="application/json",
)

assert (
OpenIDClient.wait_for_oidc_server(config, make_logger)
== "https://example.com"
)

def test_construct_oidc_client_fail(self):
"""Verfiy .construct_oidc_client() failure mode"""

Expand Down
25 changes: 15 additions & 10 deletions lib/pbench/test/unit/server/test_shell_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,7 @@ def test_find_the_unicorn(monkeypatch, make_logger):

@staticmethod
@pytest.mark.parametrize("user_site", [False, True])
@pytest.mark.parametrize("oidc_conf", [False, True])
def test_main(
monkeypatch, make_logger, mock_get_server_config, user_site, oidc_conf
):
def test_main(monkeypatch, make_logger, mock_get_server_config, user_site):
called = []

def find_the_unicorn(logger: logging.Logger):
Expand All @@ -89,10 +86,8 @@ def init_indexing(*args, **kwargs):
def wait_for_oidc_server(
server_config: PbenchServerConfig, logger: logging.Logger
) -> str:
if oidc_conf:
return "https://oidc.example.com"
else:
raise OpenIDClient.NotConfigured()
called.append("wait_for_oidc_server")
webbnh marked this conversation as resolved.
Show resolved Hide resolved
return "https://oidc.example.com"

commands = []

Expand All @@ -117,6 +112,7 @@ def run(args, cwd: Optional[str] = None) -> subprocess.CompletedProcess:
expected_called += [
"wait_for_uri(sqlite:///:memory:,120)",
"wait_for_uri(http://elasticsearch.example.com:7080,120)",
"wait_for_oidc_server",
"init_indexing",
]
assert called == expected_called
Expand Down Expand Up @@ -211,8 +207,17 @@ def init_db(*args, **kwargs) -> int:
assert ret_val == 1

@staticmethod
@pytest.mark.parametrize(
"exc",
[
Exception("oidc exception"),
OpenIDClient.NotConfigured,
OpenIDClient.ServerConnectionTimedOut,
OpenIDClient.ServerConnectionError,
],
)
def test_main_wait_for_oidc_server_exc(
monkeypatch, make_logger, mock_get_server_config
monkeypatch, make_logger, mock_get_server_config, exc
):
def immediate_success(*args, **kwargs):
pass
Expand All @@ -223,7 +228,7 @@ def wait_for_oidc_server(
server_config: PbenchServerConfig, logger: logging.Logger
) -> str:
called[0] = True
raise Exception("oidc exception")
raise exc

monkeypatch.setattr(shell.site, "ENABLE_USER_SITE", False)
monkeypatch.setattr(shell, "wait_for_uri", immediate_success)
Expand Down
2 changes: 1 addition & 1 deletion server/pbenchinacan/load_keycloak.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ KEYCLOAK_REDIRECT_URI=${KEYCLOAK_REDIRECT_URI:-"http://localhost:8080/*"}
ADMIN_USERNAME=${ADMIN_USERNAME:-"admin"}
ADMIN_PASSWORD=${ADMIN_PASSWORD:-"admin"}
# These values must match the options "realm" and "client in the
# "openid-connect" section of the pbench server configuration file.
# "openid" section of the pbench server configuration file.
REALM=${KEYCLOAK_REALM:-"pbench-server"}
CLIENT=${KEYCLOAK_CLIENT:-"pbench-client"}

Expand Down