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

Configurable restrictions for user list/detail view. #228

Merged
merged 5 commits into from
Oct 17, 2023

Conversation

rhyw
Copy link
Contributor

@rhyw rhyw commented Oct 9, 2023

An attempt to resolve #209

@rhyw
Copy link
Contributor Author

rhyw commented Oct 9, 2023

I added several test cases, which makes sure once configured in settings, corresponding user list/detail view are no longer accessible. However I was unable to login using the test client in test_views.py. I tried with below forms:

self.client.login(username="foo", password="xxx")

and

self.client.post("/auth/login/", {"username": "foo", "password": "xxx"})

Neither worked. I failed to find a neat way to do this.

Also after looked into the test_views.py, I believe the cases in TestAuthView are not doing what it should be doing. The auth(login/logout) is not covered in tests. I'd appreciate if someone tells how I could authenticate using the test client, since the cases for logged in user access are needed as well.

PTAL.

@rohanpm
Copy link
Member

rohanpm commented Oct 9, 2023

I added several test cases, which makes sure once configured in settings, corresponding user list/detail view are no longer accessible. However I was unable to login using the test client in test_views.py. I tried with below forms:

self.client.login(username="foo", password="xxx")

I believe the above works if given a correct username and password, but no users seem to exist by default.

The following worked for me, to create a user before authenticating with it:

user = User.objects.create(username='testuser')
user.set_password('testpass')
user.save()

assert self.client.login(username="testuser", password="testpass")

kobo/django/views/generic.py Outdated Show resolved Hide resolved
kobo/django/views/generic.py Outdated Show resolved Hide resolved
kobo/django/views/generic.py Outdated Show resolved Hide resolved
@rhyw
Copy link
Contributor Author

rhyw commented Oct 10, 2023

I added several test cases, which makes sure once configured in settings, corresponding user list/detail view are no longer accessible. However I was unable to login using the test client in test_views.py. I tried with below forms:

self.client.login(username="foo", password="xxx")

I believe the above works if given a correct username and password, but no users seem to exist by default.

The following worked for me, to create a user before authenticating with it:

user = User.objects.create(username='testuser')
user.set_password('testpass')
user.save()

assert self.client.login(username="testuser", password="testpass")

I guess my mistake was that I created the user and specified the password at the same time, something like this:

create(username=username, password=password)

which just won't work! I'll try use set_password explicitly, thanks!

@rhyw rhyw force-pushed the user-restrictions branch 3 times, most recently from cd56958 to fe0d42f Compare October 10, 2023 07:31
Copy link
Member

@rohanpm rohanpm left a comment

Choose a reason for hiding this comment

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

Could you please arrange for one other person who also works on openscanhub to post a +1 on this so that I know you guys have agreed on the solution?

@rhyw
Copy link
Contributor Author

rhyw commented Oct 17, 2023

@kdudka would you mind give a look when got time? Thanks.

Copy link
Contributor

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

I confirm it works as expected. Thanks!

@rohanpm rohanpm merged commit 3e54ffa into release-engineering:master Oct 17, 2023
15 checks passed
@rhyw rhyw deleted the user-restrictions branch October 18, 2023 01:48
rhyw added a commit to rhyw/openscanhub that referenced this pull request Oct 20, 2023
With this update, the user list/detail view will be accessible only to
staff users, this ensures the app meets compliance requirements.

Related: release-engineering/kobo#228
Related: release-engineering/kobo#209
Related: https://issues.redhat.com/browse/OSH-78
rhyw added a commit to openscanhub/openscanhub that referenced this pull request Oct 24, 2023
With this update, the user list/detail view will be accessible only to
staff users, this ensures the app meets compliance requirements.

Related: release-engineering/kobo#228
Related: release-engineering/kobo#209
Related: https://issues.redhat.com/browse/OSH-78
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrict access to user list/detail view
3 participants