Skip to content
This repository has been archived by the owner on Dec 16, 2021. It is now read-only.

Added seperate token file #267

Closed
wants to merge 14 commits into from
Closed

Conversation

Kruhlmann
Copy link

@Kruhlmann Kruhlmann commented Apr 9, 2020

Closes #262

Moves the token to a seperate file (~/.config/cordless/token).

Accepts -token-file parameter for custom token file location (same was as the custom config location).

Tested on Manjaro Linux x86_64 only and has not been tested with multiple accounts.

@Bios-Marcel
Copy link
Owner

The "Account" fields of th configuration would have to be moved into the token file as well

@Kruhlmann
Copy link
Author

Kruhlmann commented Apr 9, 2020

The account information has been moved from the config file to accounts.json.

I also added -accounts-file to the launch parameters.

@Bios-Marcel
Copy link
Owner

Is there any benefit in not having the token in the accounts file?

@Kruhlmann
Copy link
Author

No, that's probably a good idea.

app/app.go Outdated Show resolved Hide resolved
config/account.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved

// SetAccountsFile sets the accounts file path cache to the
// entered value
func SetAccountsFile(accountsFilePath string) error {
Copy link
Owner

Choose a reason for hiding this comment

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

This code was copied and we should avoid duplication. If there's a bug in this code, it most likely won't be fixed for config. The same goes for all other code that was copied.

config/account.go Outdated Show resolved Hide resolved
@Bios-Marcel
Copy link
Owner

I still kinda dislike the fact that we'll log-out everyone that's arleady logged in. And this will also clear everyones already created cordless accounts.

@Kruhlmann
Copy link
Author

I agree with that, but I think it's a better solution than having permanent migration code.

Then the question becomes whether or not it's worth it to log everyone out and clear their accounts in order to accomodate this feature.

@Bios-Marcel
Copy link
Owner

Well, maybe the migration code doesn't need to be permanent

config/account.go Outdated Show resolved Hide resolved
@Bios-Marcel
Copy link
Owner

Hey, I am sorry for slacking for so long, I really haven't had much motivation lately. I'll try looking at this again over the next days.

@Bios-Marcel
Copy link
Owner

Then the question becomes whether or not it's worth it to log everyone out and clear their accounts in order to accomodate this feature.

I am still kinda unsure with this. I really want to avoid logging everyone out. And like said, migration code is somewhat annoying too. But what if we added a version number to the configuration and added a manual configuration step. So that when you start cordless, says There are breaking changes, run "cordless --migrate". So we could avoid doing it on every startup and have a separate package that duplicates the code without having to be careful with the maincode.

@Bios-Marcel
Copy link
Owner

Closing this, as I am shutting down the project either way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move token out of configuration file
2 participants