APP-7007: implement DisableAuthService CLI command #4686
Conversation
viambot
added
the
safe to test
| printf(c.App.Writer, yellow, fmt.Sprintf("You are trying to disable auth service for organization ID %s. "+ | ||
| "Once disabled, this will remove and permanently delete all white-labeled login flows for any applications "+ | ||
| "associated with your organization ID\n", args.OrgID)) | ||
| printf(c.App.Writer, yellow, "If you wish to continue, please type \"disable\":") |
| @@ -371,6 +371,25 @@ func TestEnableAuthServiceAction(t *testing.T) { | |||
| test.That(t, out.messages[0], test.ShouldContainSubstring, "enabled auth") | |||
| } | |||
|
|
|||
| func TestDisableAuthServiceAction(t *testing.T) { | |||
| disableAuthServiceFunc := func(ctx context.Context, in *apppb.DisableAuthServiceRequest, opts ...grpc.CallOption) ( | |||
There was a problem hiding this comment.
Any way to validate
- the empty org ID
- the valid org ID followed by an abort
- the valid org ID followed by a DISABLE
? If not easy lmk - but would hope to assert that stuff in code in addition to our manual tests
There was a problem hiding this comment.
i dont know if there is an easy way to cases 2 and 3 here, @purplenicole730 do u know how i could test this? but i did add a test case for the empty org ID case
There was a problem hiding this comment.
I'm not too sure, since I'm not too familiar with the Before capability (which I'm assuming is what DISABLE is from). My best guess is to see how the other functions that have Before capabilities are tested.
There was a problem hiding this comment.
If you can't figure it out after searching on the internet/our code, then at least include this test in the description so that we know you manually tested it.
There was a problem hiding this comment.
I couldn't find any existing tests for the Before action , but I added my manual testing in the description!
| }, | ||
| }, | ||
| { | ||
| Name: "auth-service", |
There was a problem hiding this comment.
Is this deleting the existing "auth-service"? Ideally wouldn't remove here - we have a separate ticket in the epic for unwinding / removing the existing stuff.
There was a problem hiding this comment.
Agreed. Otherwise, this becomes a breaking change
There was a problem hiding this comment.
no, this was a mistake i made when merging the enable auth service ticket earlier, i accidentally ended up having 2 "auth-service" subcommands (from this epic), so im fixing this and putting it in the right spot here. the existing "auth-service" stuff is still in this file.
@maxhorowitz If you don't mind, can you add all these comments into one review so that it's easier to follow, and it doesn't spam others' emails 😅 ? |
Co-authored-by: Max Horowitz <[email protected]>
Co-authored-by: Max Horowitz <[email protected]>
viambot
added
safe to test
| }, | ||
| }, | ||
| { | ||
| Name: "auth-service", |
There was a problem hiding this comment.
Agreed. Otherwise, this becomes a breaking change
cli/client.go
Outdated
| "Once disabled, all custom auth views and emails will be removed from your organization's (%s) " + " | ||
| "OAuth applications and permanently deleted.\n", args.OrgID)) |
There was a problem hiding this comment.
The image in the PR description doesn't seem to match this text. Please update. I'd like to see it since I'm not sure what the second %s value is.
viambot
added
safe to test
Co-authored-by: Max Horowitz <[email protected]>
Co-authored-by: Max Horowitz <[email protected]>
viambot
added
safe to test
Co-authored-by: Max Horowitz <[email protected]>
viambot
added
safe to test
viambot
added
safe to test
cli/app.go
Outdated
| { | ||
| Name: "disable", | ||
| Usage: "disable auth-service for OAuth applications", | ||
| UsageText: createUsageText("disable", []string{generalFlagOrgID}, true), |
There was a problem hiding this comment.
I just realized, why is otheroptions marked as true? The only option is the required one, so this should be set to false.
cli/app.go
Outdated
| { | ||
| Name: "disable", | ||
| Usage: "disable auth-service for OAuth applications", | ||
| UsageText: createUsageText("disable", []string{generalFlagOrgID}, true), |
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
Ticket:
Manual testing:
abort option
disable option (didn't have an org with auth service enabled yet, but the rpc response is expected)