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

Jump to bottom

fix: Add unrevoke to the at_onboarding_cli #682

Merged
merged 22 commits into from
Oct 15, 2024
Merged

fix: Add unrevoke to the at_onboarding_cli #682

merged 22 commits into from
Oct 15, 2024

Conversation

sitaram-kalluri
Copy link
Member

@sitaram-kalluri sitaram-kalluri commented Oct 3, 2024 •
edited by gkc
Loading

Closes #627 and #628

This PR includes everything from #670

- What I did

  • Add "unrevoke" to auth_cli commands which will restore the revoked enrollmentId.
  • Add "delete" to auth_cli commands which will permanently delete enrollment requests (only denied/revoked enrollments may be deleted)
  • Made various changes (replacing exit with throw, etc) to ensure that the tests in enrollment_cli_commands_test.dart behave as they should

- How I did it

  • Introduced 'delete' as an option in the at_activate argParser
  • Introduced a delete-command parser which considers enrollmentId mandatory
  • Introduce a new method deleteDeniedEnrollment which sends a delete request to the server and parses the server response.
  • Add "unrevoke" command to auth_cli_args list.
  • Add "unrevoke" method in auth_cli which will do the necessary work to unrevoke the enrollment Id.

- How to verify it

  • Added an End-to-End test in "enrollment_cli_commands_test.dart" file which covers unrevoke functionality.
  • Manual testing has been done.
  • To test the changes, send an enrollment request then deny/approve+revoke it. Then run at_activate with the delete option which requires the atsign and enrollmentId. The enrollment should be deleted.

@sitaram-kalluri
Copy link
Member Author

The functional tests in at_onboarding_cli are failing due to "enrollment_cli_commands_test.dart", which requires the unrevoke functionality on the secondary server in production. Currently, the unrevoke feature is only available in the canary.

@gkc gkc mentioned this pull request Oct 8, 2024
Merged
@gkc
Copy link
Contributor

gkc commented Oct 8, 2024

@sitaram-kalluri when are you planning to promote canary to prod?

@sitaram-kalluri
Copy link
Member Author

@sitaram-kalluri when are you planning to promote canary to prod?

@gkc : We released the latest version to canary last week? Shall we do a prod release today ?

@gkc
Copy link
Contributor

gkc commented Oct 9, 2024

Yes; I will do a prod release now

@gkc
Copy link
Contributor

gkc commented Oct 9, 2024

@sitaram-kalluri Or - does this bug fix need to go out to canary first?

@sitaram-kalluri
Copy link
Member Author

@sitaram-kalluri Or - does this bug fix need to go out to canary first?

@gkc : Yeah, Let us first release the bug fix to canary and then do a prod release.

gkc added 8 commits October 9, 2024 11:46
@gkc
Merge remote-tracking branch 'origin/trunk' into at_activate_remove_d…
fcdaae7 
…enied
@gkc
docs: at_onboarding_cli: add to CHANGELOG for delete command
29b5c7c
@gkc
Merge remote-tracking branch 'origin/trunk' into unrevoke_at_onboardi…
6de0496 
…ng_cli_changes
@gkc
docs: at_onboarding_cli: add to CHANGELOG for unrevoke command
5407f7f
@gkc
chore: dart format
57a066c
@gkc
fix: added parameter introduced by merge from trunk
18e37fa
@gkc
fix: removed state check from auth_cli.createAtClient as it was cau…
46833f3 
…sing unit tests to bomb

fix: removed exit statements from various parts of `auth_cli.dart` as they were causing the unit tests job to apparently succeed even though some tests were failing
@gkc
fix: use auth_cli.wrappedMain in enrollment_cli_commands_test.dart so…
8795894 
… that exceptions flow through to the tests
@gkc
Copy link
Contributor

gkc commented Oct 9, 2024

@sitaram-kalluri FYI please review the changes I've made in my last few commits starting with this one

@gkc gkc removed the request for review from murali-shris October 9, 2024 11:25
@sitaram-kalluri
Copy link
Member Author

@sitaram-kalluri FYI please review the changes I've made in my last few commits starting with this one

Sure Gary

@gkc gkc mentioned this pull request Oct 9, 2024
Merged
gkc added 4 commits October 9, 2024 12:39
@gkc
Merge remote-tracking branch 'origin/at_activate_remove_denied' into …
63eeb69 
…unrevoke_at_onboarding_cli_changes

# Conflicts:
#	packages/at_onboarding_cli/CHANGELOG.md
#	packages/at_onboarding_cli/lib/src/cli/auth_cli.dart
#	packages/at_onboarding_cli/lib/src/cli/auth_cli_args.dart
@gkc
fix: replaced another exit statement with a throw - this time in …
0c16ffd 
…onboarding_util.dart
@gkc
chore: dart format
ba133dd
@gkc
build: update at_onboarding_cli version to 1.8.0 ready for publishing…
53c3288 
… once we've merged to trunk
@gkc gkc marked this pull request as draft October 9, 2024 14:47
@gkc
Copy link
Contributor

gkc commented Oct 9, 2024

Marking as draft until this PR has been completed, reviewed, merged and released first to canary and then to prod

@sitaram-kalluri sitaram-kalluri marked this pull request as ready for review October 15, 2024 09:49
@sitaram-kalluri
Copy link
Member Author

Marking as draft until this PR has been completed, reviewed, merged and released first to canary and then to prod

The changes are released into the prod with version at_secondary_server v3.0.51. All the functional tests are passing now and therefore, marking the PR ready for review.

gkc
gkc approved these changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @srieteja and @sitaram-kalluri

@gkc
Copy link
Contributor

gkc commented Oct 15, 2024

@sitaram-kalluri Can you review, approve and merge this please, as I was the last committer.

@murali-shris murali-shris merged commit ea41c56 into trunk Oct 15, 2024
11 checks passed
@murali-shris murali-shris deleted the unrevoke_at_onboarding_cli_changes branch October 15, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@murali-shris murali-shris murali-shris approved these changes

@gkc gkc gkc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

at_activate adding option to un-revoke/enable revoked atKeys
4 participants
@sitaram-kalluri @gkc @murali-shris @srieteja