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

Support auth for multiple workspaces #121

Open
mikeschinkel opened this issue Aug 29, 2022 · 7 comments
Open

Support auth for multiple workspaces #121

mikeschinkel opened this issue Aug 29, 2022 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@mikeschinkel
Copy link

mikeschinkel commented Aug 29, 2022

Is your feature request related to a problem? Please describe.

I would like to use this with multiple different Slack URLs without having keep logging in to each different one.

Describe the solution you'd like
When saving the auth associate it with the URL in the file in which you save it. Then when running slackdump with no args and it gets to the Slack Workspace Name prompt, it would list the ones authorized but also allow selecting "Authenticate a new Workspace."

This would probably make the -auth-reset option mostly moot.

Describe alternatives you've considered
I can't think of one other than manually logging in and out.

Additional context
I'd be happy to consider writing this feature — I am professionally working as a Go developer — but I'd want to know that you would bless the idea and offer your tips in advance for how you would want it done so that I don't waste time producing a PR you would not accept.

@mikeschinkel mikeschinkel changed the title Support auth for multiple Workspace names Support auth for multiple workspaces Aug 29, 2022
@rusq
Copy link
Owner

rusq commented Aug 30, 2022

Hi @mikeschinkel , thank you for offering a hand with this. I think it's a great suggestion. Right now, the credentials are stored in the provider.bin file, as you might have checked already, it's an encrypted JSON file, containing marshalled "auth.SlackCreds". It shouldn't be a problem to add a "Workspace" member variable to it, and then read this file on start and offer user a choice of saved credentials to load.

The only dodgy case I could think of is when the credentials are provided manually via ".env" (or secrets.txt) which won't give the workspace name. Right now the credentials are resolved the following way, TL;DR - when credentials are provided via the ".env", slackdump ignores providers.bin. I can think of two possible solutions:

  1. Save it in provider.bin with a special Workspace name, i.e. "$default$", or empty string "" and treat is as a special case in the code
  2. do not offer the choice of credentials at all if ".env" is populated.

I'm more inclined towards approach (1), as this will give a user a choice, but I already don't like having the special case :)

Please let me know what you think?

@mikeschinkel
Copy link
Author

@rusq — Thank you for the quick reply.

Yes, after posting this I ran into a problem with the API returning invalid_arguments so I loaded up the code in GoLand to debug it and found ~/Library/Caches/slackdump/provider.bin on my macOS.

That made me think maybe a solution to this would be to just have different .bin files for each Slack workspace, e.g.:?

That would allow backward compatibility to provider.bin, and it would allow switching the default by copying one of the named providers to provider.com.

If you agree this makes sense, it would probably mean adding a few more switches, maybe:

  • -set-ws <ws> — Would copy provider-<ws>.bin to provider.bin
  • -ws <ws> — Would use provider-<ws>.bin instead of provider.bin

There may be more, but that's all I got at the moment. Thoughts?

@rusq
Copy link
Owner

rusq commented Aug 31, 2022

Looks good, @mikeschinkel.

I think it would make sense to implement a workspace manager as a separate command, as user might need a way to reset auth for one or more workspaces, it could be hard for some to find the cache folder and remove the file manually. Having several make more flexible managing the manually tho, for those who decided to find and remove them manually, or even backup.

The only problem I have with extending the command line flags at the moment is that it is already a big mess! It is a mix of mode and option control flags - they can choose the behaviour or adjust some parameter. Before implementing this, I want to split the modes into commands, using the same approach as used in go tool. I have started some work on this couple of days ago.

So, instead of -set-ws it would be slackdump workspace -set bar.
And in other modes, i.e. export, user would be able to choose with `slackdump -ws foo export -type mattermost".

I suggest that the work to implement commands be completed first, and then the workspace management can be implemented. I was planning to do that as a matter of priority, as it blocks other issues as well, like #44 or #67 - they were hanging there for a while now.

@mikeschinkel
Copy link
Author

mikeschinkel commented Sep 1, 2022

I think it would make sense to implement a workspace manager as a separate command

Looking at slackdump --help I only see switches. I have v2.12. Are there commands?

I think commands are much better, and I favor Cobra for that, but I did not think your CLI architecture was designed around commands, so I didn't want to proposed that. Besides, revamping the entire thing would be a lot more scope than the functionality around just profile switching.

That said, I'm all for commands but I don't want to sign up for more than I can deliver in my free time.

it could be hard for some to find the cache folder and remove the file manually.

In what I proposed I didn't see a reason anyone would need more than the switches I proposed. What are some use-cases that I did not envision?

The only problem I have with extending the command line flags at the moment is that it is already a big mess!
I suggest that the work to implement commands be completed first, and then the workspace management can be implemented

But do getting commands in place really need to be a blocker? The functionality is orthogonal; the switches-vs-commands are just the UI layer to allow a user to control the functionality.

Wouldn't it be just as easy to change the proposed functionality from switches to commands once you get to that (set of) task(s)?

I also might be up for helping with commands after doing the profile switcher, but I'd prefer to keep the scope of each small so I might actually get at least one of them done in the near term.

Respectfully, of course. 🙂

@rusq
Copy link
Owner

rusq commented Sep 1, 2022

Hi @mikeschinkel

Apologies if I made an impression of wanting to chuck in the commands implementation in this issue, it was not the intention. Of course it's a separate issue and out of scope for this one!

Looking at slackdump --help I only see switches. I have v2.12. Are there commands?
No, not yet :) You are correct regarding that the Slackdump architecture is not built around "commands", and that's why I mentioned that there's a lot of flags that control a behaviour, and I want to change that in nearest future.

You convinced me regarding postponing the implementation of commands in favour of workspace switching. I like the approach that you have suggested with -ws-set and -ws, so I think we can go ahead with that.

I suggest that the switching functionality is to be implemented first, and then "UI" is updated to accomodate for it.

@rusq rusq added the enhancement New feature or request label Sep 1, 2022
@mikeschinkel
Copy link
Author

@rusq — 

Very cool.

So here's my plan/schedule. I just left my most recent contract and will be starting a new one Sept 19. I have to finish a project for a side client, hopefully I can do that in the next few days. Then I hope I can get the change done in 1 day because I need to take a vacation from coding for at least 10 days.

I'll follow up soon, hopefully.

@rusq
Copy link
Owner

rusq commented Sep 5, 2022

Thanks Michael! Don't stress out if you won't finish in a day, better take some rest before the new challenges. I can continue from where you stop

@rusq rusq added this to the v2.3.0 milestone Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants