refactor lib to allow support for other Okta applications

[switj]

Hello @nickatsegment

We've been looking at other Okta integrations, in particular Kubernetes with Implicit OIDC, with some refactoring the OktaClient could be re-used for these other auth flows.

What are your thoughts about a new major release that would have a more clear boundary between Okta auth and AWS SAML app auth flows?

I'm thinking a structure like:

                                      <ExecCmd>
                                          |
                                  [AwsSamlProvider]
                                          |
                                    [OktaClient]

Where ExecCmd (or any other command using the provider) would remain mostly unchanged, AwsSamlProvider would be responsible for all of the AWS SAML specific logic and for caching aws credentials.

OktaClient would be responsible for authenticating the Okta session via api/v1/authn and any MFA support. The client would also be responsible for caching the Okta session credentials.

We have some development time set aside for this work and are happy to make the changes. I would appreciate your design input so that the changes can be part of the aws-okta in future releases.

[nickatsegment]

I am really into the idea of a refactor and have been thinking about it for a very long time. I have many thoughts.

Considering we're still pre-v1, according to golang compatibility guidelines, we can technically change the API however we want. That said, I've been a stickler in the past about changing the API unnecessarily, because it always should prompt a new release and it means rebasing any PRs in flight. In this case though, a breaking change seems pretty necessary. I think we should stick with v0.x until we are dead certain that the API is good and stable, and then release a v1.

I think this would be a lot easier if we versioned the client separately from the libs. If we're introducing a new consumer of the lib API that might have changes that wouldn't affect the CLI, then it'd be nice to not have to cut CLI releases for those versions. I've been reading up on multi-module go repos, but haven't taken the plunge to actually use them and don't know how well they work. It might be safer to have the lib in its own separate repo, but that will probably be annoying. Thoughts?

It would be nice if we could do the development of the new lib in parallel. Here's how I'd propose doing that:

1. Create a new package/dir lib2 (this is a bad name; real name TBD). This might be many packages.
2. Copy existing lib code into lib2/internal/lib_orig
3. In lib2, gradually expose/refactor new API, importing lib_orig as necessary.

• Gradually push the CLI to using these new APIs. The CLI should not change functionally, so new releases should not need to be made, but we could cut pre-releases. Ultimately, we want to do exactly 1 minor release of the client to start using lib2 in its entirety.
• When nothing is referencing a symbol in lib, we can mark it as deprecated, and it will be removed in v1.
• Simultaneously, you can start consuming lib2 in your new code to make sure it's fit to purpose.

4. Once all of lib is present in lib2, we can mark lib as entirely deprecated and cut a minor release of the CLI.
5. At some point in the future, release v1.0 and remove all of lib, leaving only lib2. This probably won't require a new CLI release, but it might feel good to do so :)

The main idea here is to minimize coordination and backporting efforts. We don't want this refactoring effort to impede bugfixes or features on the existing codebase. And we also don't want the work to languish on a branch that drifts from master. Does this make sense? Or seem too fiddly?

[switj]

Your suggested approach sounds like the best way forward to get these changes in use.

The only item that I don't understand is

2. Copy existing lib code into lib2/internal/lib_orig

Could you elaborate more on the benefits of doing this?

We have some ideas on the changes to the provider and client. Is there anything else that you wanted to refactor before we make the new packages in lib2 public?

Once the refactor is public we can start using it to create our own client for authenticating k8s via oidc/okta.

We'll PR the provider and client changes by the end of the week.

[nickatsegment]

Could you elaborate more on the benefits of doing this?

It's probably not essential actually. It would just show more clearly how much of the codebase has been migrated to lib2.

Is there anything else that you wanted to refactor before we make the new packages in lib2 public?

I don't really like the looseness of the pervasive Profiles type; it should probably be nicely typed structs.

And in general, the business logic and UI are too intertwined: e.g., there are CLI prompts and env var lookups happening in the lib code.

If you're intending to use this from an automated client you probably would have had to clean those up anyway, so maybe those are obvious points.

I realize we have almost 0 automated tests currently, but adding some to show that things work the way they should would be very much appreciated :) Also docstrings.

I had a thought on the CLI vs API versioning: maybe we use the top-level module to mean the CLI and then the (new) API would be a sub go module, so tags would look like v0.27.0 (meaning the version of the aws-okta CLI) and libs2/v0.1.0 would mean the API. This might require some playing around to get the CI right.

[nickatsegment]

I've pinned this and #236 to increase visibility. Hopefully this will make the refactor smoother, and avoid extensive rebasing.

[stale]

This issue has been automatically marked stale because it has not had any activity in the last 60 days. If no further activity occurs within 7 days, it will be closed. Closed does not mean "never", just that it has no momentum to get accomplished any time soon.
See CONTRIBUTING.md for more info.

[stale]

Closing due to staleness. Closed does not mean "never", just that it has no momentum to get accomplished any time soon.
See CONTRIBUTING.md for more info.