Skip to content

[fix] Fixed project configuration for RADIUS module #359

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

Jump to bottom
Merged
pandafy merged 2 commits into from
Sep 13, 2024
Merged

[fix] Fixed project configuration for RADIUS module #359

pandafy merged 2 commits into from
Sep 13, 2024

Conversation

@pandafy
Copy link
Member

@pandafy pandafy commented Sep 11, 2024 •
edited
Loading

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Description of Changes

Fixed bug with RADIUS API for user registration.

The bug was reported in https://github.com/orgs/openwisp/discussions/921

TODOS

  • Add automated test

@pandafy pandafy marked this pull request as draft September 11, 2024 12:04
@nemesifier nemesifier added the bug Something isn't working label Sep 11, 2024
@pandafy pandafy force-pushed the fix-radius-registration branch from ff94467 to ded84fd Compare September 11, 2024 13:00
nemesifier
nemesifier reviewed Sep 11, 2024
View reviewed changes
Copy link
Member

@nemesifier nemesifier 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 the fix works! I think just 1 test will be enough, make sure it fails without the fix please @pandafy 🙏

@pandafy pandafy force-pushed the fix-radius-registration branch 2 times, most recently from fadd4c4 to ba30870 Compare September 12, 2024 13:53
@pandafy pandafy marked this pull request as ready for review September 12, 2024 13:57
nemesifier
nemesifier reviewed Sep 12, 2024
View reviewed changes
tests/runtests.py Outdated

def test_radius_user_registration(self):
"""Ensure users can register using the RADIUS API."""
url = f"{self.config['api_url']}/api/v1/radius/" "organization/default/account/"
Copy link
Member

@nemesifier nemesifier Sep 12, 2024

Choose a reason for hiding this comment

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

Suggested change
url = f"{self.config['api_url']}/api/v1/radius/" "organization/default/account/"
url = f'{self.config["api_url"]}/api/v1/radius/organization/default/account/'

@pandafy
[fix] Fixed project configuration for RADIUS module
2e81ec8 
Fixed bug with RADIUS API for user registration
@pandafy pandafy force-pushed the fix-radius-registration branch from ba30870 to 2e81ec8 Compare September 12, 2024 19:34
nemesifier
nemesifier approved these changes Sep 12, 2024
View reviewed changes
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@pandafy LGTM! Let me know if you agree with my latest change.

pandafy
pandafy commented Sep 13, 2024
View reviewed changes
Copy link
Member Author

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

LGTM!

@pandafy pandafy merged commit cc94e04 into master Sep 13, 2024
@pandafy pandafy deleted the fix-radius-registration branch September 13, 2024 10:46
@3pleo
Copy link

3pleo commented Sep 17, 2024 •
edited
Loading

Thanks.
This has been tested. But following similar error pops up when "Confirm Email" link sent is clicked:

NoReverseMatch at /accounts/confirm-email/NTg:1sqSLG:FeIEip6aYPX77iN8vZ99FWKa5XWWvmRUlD2TxOK3qlY/
'admin' is not a registered namespace

find the full trace attached.
confirm_email_error.txt

Regards

@nemesifier
Copy link
Member

nemesifier commented Sep 17, 2024

Thanks. This has been tested. But following similar error pops up when "Confirm Email" link sent is clicked:

NoReverseMatch at /accounts/confirm-email/NTg:1sqSLG:FeIEip6aYPX77iN8vZ99FWKa5XWWvmRUlD2TxOK3qlY/ 'admin' is not a registered namespace

find the full trace attached. confirm_email_error.txt

Regards

@3pleo please create a new issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nemesifier nemesifier nemesifier approved these changes

Assignees

@nemesifier nemesifier

@pandafy pandafy

Labels

bug Something isn't working

Projects

Docker OpenWISP Effort
Status: Done
OpenWISP Priorities for next releases
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pandafy @3pleo @nemesifier