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

User self-registration #435

Merged
merged 50 commits into from
Jun 29, 2021
Merged

User self-registration #435

merged 50 commits into from
Jun 29, 2021

Conversation

fmigneault
Copy link
Collaborator

@fmigneault fmigneault commented Jun 11, 2021

Features / Changes

  • Changed UserStatuses.WebhookErrorStatus = 0 to UserStatuses.WebhookError = 2 to provide further
    functionalities. Migration script applies this change to existing User entries.
  • Changed the returned status value by the API routes to use the string name representation instead of the integer.
  • Changed status search query handling of GET /users path for improved search and filtering capabilities.
  • Add new UserStatuses.Pending = 4 value that can be queried by administrators.
  • Add UserPending object with corresponding table for pending approval by an administrator for some new
    self-registered user. Migration script creates the table with expected fields.
  • Add new requests under /register/users and /ui/register/users endpoints for user account self-registration.
  • Add UI view to display pending user registration details.
  • Add UI icon to indicate when a listed user is pending registration approval or email validation.
  • Disable user email self-update (when not administrator) both on the API and UI side
    whenever MAGPIE_USER_REGISTRATION_ENABLED was activated to avoid losing the confirmation of the original email.
  • Add configuration setting MAGPIE_USER_REGISTRATION_ENABLED to control whether user account self-registration
    feature should be employed.
    With it comes multiple other MAGPIE_USER_REGISTRATION_<...> settings to customize notification emails.
  • Add multiple MAGPIE_SMTP_<...> configuration settings to control connections to notification email SMTP server.
  • Add empty_missing flag to get_constant utility to allow validation against existing environment variables or
    settings that should be considered as invalid when resolved value is an empty string.
  • Add missing format for applicable url and email elements in the OpenAPI specification.
  • Add better logging options control in CLI operations.
  • Add new CLI helper send_email to test various email template generation and SMTP configurations to send emails.
  • Replace -d option of register_providers CLI operation (previously used to select database mode)
    by --db to avoid conflict with logging flags.
  • Replace -d and -l options of batch_update_users CLI operation respectively by -D and -L
    to avoid conflict with logging flags.

Bug Fixes

  • Explicitly disallow duplicate email entries, both with pre-validation and literal database values.
    Note that any duplicate email will be raised an migration script will fail. Manual cleanup of the undesired entry
    will be required, as Magpie will not be able to assume which one corresponds to the valid user to preserve.

  • Add ziggurat_foundations extensions for Pyramid directly in the code during application setup such that an INI
    configuration file that omits them from pyramid.include won't cause Magpie to break.

Other references

Implements feature DAC-117

@github-actions github-actions bot added api Something related to the API operations ci Something related to code tests, deployment and packaging cli Something related to the CLI helpers db Issues related to database connection, migration or data models doc Documentation improvements or building problem tests Test execution or additional use cases ui Something related to the UI operations or display webhook Item related to webhooks functionality. labels Jun 11, 2021
@fmigneault fmigneault force-pushed the register-user branch 2 times, most recently from 246db8b to 40cc1ea Compare June 11, 2021 21:02
@fmigneault fmigneault changed the title Register user User self-registration Jun 16, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2021

Codecov Report

Merging #435 (e97d68c) into master (1167b2c) will increase coverage by 0.67%.
The diff coverage is 77.99%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #435      +/-   ##
==========================================
+ Coverage   78.40%   79.07%   +0.67%     
==========================================
  Files          66       70       +4     
  Lines        8019     8779     +760     
  Branches     1080     1201     +121     
==========================================
+ Hits         6287     6942     +655     
- Misses       1496     1558      +62     
- Partials      236      279      +43     
Impacted Files Coverage Δ
magpie/permissions.py 92.46% <ø> (+0.79%) ⬆️
magpie/services.py 80.25% <ø> (ø)
magpie/ui/management/views.py 47.32% <58.53%> (+0.46%) ⬆️
magpie/cli/send_email.py 60.71% <60.71%> (ø)
magpie/ui/utils.py 66.45% <61.03%> (-6.89%) ⬇️
magpie/api/notifications.py 64.28% <64.28%> (ø)
magpie/cli/register_defaults.py 65.17% <66.66%> (-0.28%) ⬇️
magpie/cli/sync_resources.py 32.06% <66.66%> (+0.19%) ⬆️
magpie/cli/utils.py 69.23% <69.23%> (ø)
magpie/api/management/user/user_utils.py 96.02% <70.58%> (-1.34%) ⬇️
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eea73d2...e97d68c. Read the comment docs.

@fmigneault fmigneault marked this pull request as ready for review June 17, 2021 16:14
@fmigneault fmigneault requested review from dbyrns and ChaamC June 17, 2021 16:14
@fmigneault
Copy link
Collaborator Author

@ChaamC
Tagging you as some of the files/configs needed for T&C feature are added here.

Copy link
Contributor

@dbyrns dbyrns left a comment

Choose a reason for hiding this comment

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

Review in progress 5/74

config/magpie.ini Show resolved Hide resolved
docs/authentication.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@dbyrns dbyrns left a comment

Choose a reason for hiding this comment

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

Review in progress 19/74

dbyrns
dbyrns previously approved these changes Jun 21, 2021
Copy link
Contributor

@dbyrns dbyrns left a comment

Choose a reason for hiding this comment

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

Review completed. Great job!

magpie/models.py Show resolved Hide resolved
magpie/models.py Show resolved Hide resolved
tests/test_models.py Show resolved Hide resolved
dbyrns
dbyrns previously approved these changes Jun 23, 2021
Copy link
Contributor

@dbyrns dbyrns left a comment

Choose a reason for hiding this comment

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

Still good for me

tests/test_magpie_ui.py Outdated Show resolved Hide resolved
ChaamC
ChaamC previously approved these changes Jun 23, 2021
@fmigneault fmigneault dismissed stale reviews from ChaamC and dbyrns via b75997f June 28, 2021 20:56
@fmigneault fmigneault requested review from ChaamC and dbyrns June 28, 2021 22:07
…wngrade step + enforce non-null pending user-name and password
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Something related to the API operations ci Something related to code tests, deployment and packaging cli Something related to the CLI helpers db Issues related to database connection, migration or data models doc Documentation improvements or building problem tests Test execution or additional use cases ui Something related to the UI operations or display webhook Item related to webhooks functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants