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.

Sign up for GitHub

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

use new version of keycloak server with unit tests #74

Merged
merged 7 commits into from
Apr 16, 2024
Merged

use new version of keycloak server with unit tests #74

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1a735bc
use new version of keycloak server with unit tests
vbrik Apr 10, 2024
326a86f
put back empty attribute test
vbrik Apr 11, 2024
eed33ce
missing groups attribute
vbrik Apr 11, 2024
5e550fe
trigger tests
vbrik Apr 11, 2024
175af8e
forgot to revert another 'groups' attribute access
vbrik Apr 12, 2024
b08c286
don't rely to groups claim being in token
vbrik Apr 12, 2024
2363251
Update test_api_users.py
vbrik Apr 16, 2024
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: 5 additions & 4 deletions .github/workflows/wipac_cicd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
uses: actions/checkout@v3
with:
token: ${{ secrets.PERSONAL_ACCESS_TOKEN }}
- uses: WIPACrepo/wipac-dev-py-setup-action@v2.6
- uses: WIPACrepo/wipac-dev-py-setup-action@v3.1

py-versions:
needs: [py-setup]
Expand Down Expand Up @@ -58,10 +58,11 @@ jobs:
runs-on: ubuntu-latest
services:
keycloak:
image: quay.io/keycloak/keycloak:10.0.2
image: ghcr.io/wipacrepo/keycloak-rest-services:test-keycloak-master
env:
KEYCLOAK_USER: admin
KEYCLOAK_PASSWORD: admin
KEYCLOAK_ADMIN: admin
KEYCLOAK_ADMIN_PASSWORD: admin
CMD: start-dev
ports:
- 8080:8080
mongo:
Expand Down
11 changes: 2 additions & 9 deletions tests/test_api_users.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,6 @@ async def test_user_put(server):
with pytest.raises(Exception):
await client.request('PUT', '/api/users/test', {'loginShell': 'foo'})

await client.request('PUT', '/api/users/test', {'loginShell': ''})
ret = await krs.users.user_info('test', rest_client=krs_client)
assert ret['attributes']['loginShell'] == ''

dsschult marked this conversation as resolved.
Show resolved Hide resolved

@pytest.mark.asyncio
async def test_user_unauthorized(server):
Expand All @@ -111,15 +107,15 @@ async def test_user_unauthorized(server):
@pytest.mark.asyncio
async def test_user_inst_admin(server):
rest, krs_client, *_ = server

await krs.groups.create_group('/institutions', rest_client=krs_client)
await krs.groups.create_group('/institutions/IceCube', rest_client=krs_client)
await krs.groups.create_group('/institutions/IceCube/UW-Madison', rest_client=krs_client)

client = await rest('test', groups=['/institutions/IceCube/UW-Madison'])

client2 = await rest('test2', groups=['/institutions/IceCube/UW-Madison/_admin'])

ret = await client.request('GET', '/api/users/test')
assert ret['firstName'] == 'first'
assert ret['lastName'] == 'last'
Expand Down Expand Up @@ -202,9 +198,6 @@ async def test_username_select(server, reg_token_client):
]
invalid_usernames_put = [
'foò', # unicode
'fo=o', # invalid char
'fo o', # space
'f\'oo', # quote
Comment on lines -205 to -207
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These usernames can no longer be created in newer keycloak versions, which breaks test_user_put_invalid()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it break on client = await rest(username)? I guess then it's useful to split this into two lists, the second of which goes to test_invalid_usernames() and test_username_invalid().

]

@pytest.mark.parametrize('username', valid_usernames_put)
Expand Down
22 changes: 11 additions & 11 deletions tests/test_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ async def test_get_group_info_from_id(keycloak_bootstrap):

@pytest.mark.asyncio
async def test_get_members_large(keycloak_bootstrap):
users = string.ascii_lowercase
groups = string.ascii_lowercase
users = [c*3 for c in string.ascii_lowercase]
groups = [c*3 for c in string.ascii_lowercase]
for u in users:
await krs.users.create_user(u, 'first', 'last', f'{u}@email', rest_client=keycloak_bootstrap)
for g in groups:
Expand All @@ -53,16 +53,16 @@ async def test_get_members_large(keycloak_bootstrap):
cache = user_mgmt.cache.KeycloakGroupCache(krs_client=keycloak_bootstrap)
for g in groups:
ret = await cache.get_members(f'/{g}')
assert ret == list(users)
assert ret == users

await krs.groups.remove_user_group('/a', 'a', rest_client=keycloak_bootstrap)
ret = await cache.get_members('/a')
assert ret == list(users)
await krs.groups.remove_user_group('/aaa', 'aaa', rest_client=keycloak_bootstrap)
ret = await cache.get_members('/aaa')
assert ret == users

@pytest.mark.asyncio
async def test_invalidate_one(keycloak_bootstrap):
await krs.groups.create_group('/foo', rest_client=keycloak_bootstrap)
await krs.users.create_user('testuser', 'first', 'last', 'email', rest_client=keycloak_bootstrap)
await krs.users.create_user('testuser', 'first', 'last', 'email@email', rest_client=keycloak_bootstrap)
await krs.groups.add_user_group('/foo', 'testuser', rest_client=keycloak_bootstrap)

cache = user_mgmt.cache.KeycloakGroupCache(krs_client=keycloak_bootstrap)
Expand All @@ -80,7 +80,7 @@ async def test_invalidate_one(keycloak_bootstrap):
@pytest.mark.asyncio
async def test_invalidate_all(keycloak_bootstrap):
await krs.groups.create_group('/foo', rest_client=keycloak_bootstrap)
await krs.users.create_user('testuser', 'first', 'last', 'email', rest_client=keycloak_bootstrap)
await krs.users.create_user('testuser', 'first', 'last', 'email@email', rest_client=keycloak_bootstrap)
await krs.groups.add_user_group('/foo', 'testuser', rest_client=keycloak_bootstrap)

cache = user_mgmt.cache.KeycloakGroupCache(krs_client=keycloak_bootstrap)
Expand All @@ -97,7 +97,7 @@ async def test_invalidate_all(keycloak_bootstrap):

@pytest.mark.asyncio
async def test_list_users(keycloak_bootstrap):
await krs.users.create_user('testuser', 'first', 'last', 'email', rest_client=keycloak_bootstrap)
await krs.users.create_user('testuser', 'first', 'last', 'email@email', rest_client=keycloak_bootstrap)

cache = user_mgmt.cache.KeycloakUserCache(ttl=1, krs_client=keycloak_bootstrap)

Expand All @@ -106,7 +106,7 @@ async def test_list_users(keycloak_bootstrap):

@pytest.mark.asyncio
async def test_get_user(keycloak_bootstrap):
await krs.users.create_user('testuser', 'first', 'last', 'email', rest_client=keycloak_bootstrap)
await krs.users.create_user('testuser', 'first', 'last', 'email@email', rest_client=keycloak_bootstrap)

cache = user_mgmt.cache.KeycloakUserCache(ttl=1, krs_client=keycloak_bootstrap)

Expand All @@ -115,7 +115,7 @@ async def test_get_user(keycloak_bootstrap):

@pytest.mark.asyncio
async def test_get_users(keycloak_bootstrap):
await krs.users.create_user('testuser', 'first', 'last', 'email', rest_client=keycloak_bootstrap)
await krs.users.create_user('testuser', 'first', 'last', 'email@email', rest_client=keycloak_bootstrap)

cache = user_mgmt.cache.KeycloakUserCache(ttl=1, krs_client=keycloak_bootstrap)

Expand Down
6 changes: 3 additions & 3 deletions user_mgmt/handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ async def is_associate(self, experiment, username):

def is_super_admin(self):
"""Is the current user a super admin?"""
return '/admin' in self.auth_data['groups']
return '/admin' in self.auth_data.get('groups', [])

async def get_admins(self, group_path):
ret = await self.group_cache.get_members(group_path+'/_admin')
Expand Down Expand Up @@ -110,7 +110,7 @@ async def get_admin_groups(self):
if self.is_super_admin(): # super admin - all groups
admin_groups = await self.group_cache.list_groups()
else:
admin_groups = [g[:-7] for g in self.auth_data['groups'] if g.endswith('/_admin')]
admin_groups = [g[:-7] for g in self.auth_data.get('groups', []) if g.endswith('/_admin')]
groups = set()
for group in admin_groups:
val = group.strip('/').split('/')
Expand All @@ -130,7 +130,7 @@ async def get_admin_institutions(self):
val = group.split('/')
insts[val[2]].append(val[3])
else:
admin_groups = [g[:-7] for g in self.auth_data['groups'] if g.endswith('/_admin')]
admin_groups = [g[:-7] for g in self.auth_data.get('groups', []) if g.endswith('/_admin')]
insts = defaultdict(list)
for group in admin_groups:
val = group.strip('/').split('/')
Expand Down
Loading