Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dbutenhof committed Jul 1, 2023
1 parent 4aefda3 commit 2e3c683
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 19 deletions.
4 changes: 3 additions & 1 deletion jenkins/run-server-func-tests
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export PB_SERVER_IMAGE_TAG=${PB_SERVER_IMAGE_TAG:-"$(cat jenkins/branch.name)"}
export PB_POD_NAME=${PB_POD_NAME:-"pbench-in-a-can_${PB_SERVER_IMAGE_TAG}"}
export PB_SERVER_CONTAINER_NAME=${PB_SERVER_CONTAINER_NAME:-"${PB_POD_NAME}-pbenchserver"}

# For functional testing, assign ADMIN role to the testadmin username
export PB_ADMIN_NAMES=${PB_ADMIN_NAMES:-testadmin}

# Note: the value of PB_HOST_IP will be used to generate the TLS certificate
# and so it (not `localhost`) must also be used to access the Pbench Server;
# otherwise, the TLS validation will fail due to a host mismatch.
Expand Down Expand Up @@ -103,7 +106,6 @@ function cleanup {
}
trap cleanup INT QUIT TERM EXIT

export PB_ADMIN_NAMES=${PB_ADMIN_NAMES:-testadmin}
server/pbenchinacan/run-pbench-in-a-can
exit_status=${?}
if [[ ${exit_status} -ne 0 ]]; then
Expand Down
9 changes: 4 additions & 5 deletions lib/pbench/server/auth/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,11 @@ def verify_auth_oidc(auth_token: str) -> Optional[User]:
audiences = token_payload.get("resource_access", {})
pb_aud = audiences.get(oidc_client.client_id, {})
roles = pb_aud.get("roles", [])
admin_users = current_app.server_config.get(
"pbench-server", "admin-role", fallback=""
)
if Roles.ADMIN.name not in roles:
users = admin_users.split(",")
if username in users:
admin_users = current_app.server_config.get(
"pbench-server", "admin-role", fallback=""
)
if username in admin_users.split(","):
roles.append(Roles.ADMIN.name)

# Create or update the user in our cache
Expand Down
8 changes: 1 addition & 7 deletions lib/pbench/test/functional/server/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,7 @@ def oidc_admin(server_client: PbenchServerClient):

def register_user(oidc_admin: OIDCAdmin, user: JSONOBJECT):
try:
response = oidc_admin.create_new_user(
username=user["username"],
email=user["email"],
password=user["password"],
first_name=user["first_name"],
last_name=user["last_name"],
)
response = oidc_admin.create_new_user(**user)
except OpenIDClientError as e:
# To allow testing outside our transient CI containers, allow the tester
# user to already exist.
Expand Down
6 changes: 3 additions & 3 deletions lib/pbench/test/unit/server/auth/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ def test_verify_auth_oidc_user_update(
assert user.roles == []
assert user.username == "dummy"

# Generate a new token with a role for the same user
# Generate a token with a new username for the same UUID
token, expected_payload = gen_rsa_token(
client_id,
rsa_keys["private_key"],
Expand Down Expand Up @@ -678,7 +678,7 @@ def test_verify_auth_oidc_user_admin(
monkeypatch.setattr(Auth, "oidc_client", oidc_client)
server_config._conf.set("pbench-server", "admin-role", "friend,dummy,admin")

app = Flask("test-verify-auth-oidc-user-update")
app = Flask("test-verify-auth-oidc-user-admin")
app.logger = make_logger
app.server_config = server_config
with app.app_context():
Expand All @@ -688,7 +688,7 @@ def test_verify_auth_oidc_user_admin(
assert user.roles == ["ADMIN"]
assert user.username == "dummy"

# Generate a new token with a role for the same user
# Generate a token with a role and new username for the same UUID
token, expected_payload = gen_rsa_token(
client_id,
rsa_keys["private_key"],
Expand Down
7 changes: 4 additions & 3 deletions server/lib/config/pbench-server-default.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ admin-email=%(user)s@localhost
mailto=%(admin-email)s
mailfrom=%(user)s@localhost

# A list of usernames to be given ADMIN access on the server. These are SSO ID
# provider usernames matched against decrypted OIDC2 authorization tokens. If
# not specified, no users have ADMIN access. NOTE: this is a temporary measure
# A comma-separated list of OIDC usernames with no spaces. These usernames
# will be granted ADMIN access on the server. These are OIDC ID provider
# usernames matched against decrypted authorization tokens. If no usernames
# are specified, no users have ADMIN access. NOTE: this is a temporary measure
# until we work out Keycloak / LDAP roles.
#admin-role=user1,user2

Expand Down

0 comments on commit 2e3c683

Please sign in to comment.