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

Add debug CLI config that prevents logging from taking out values for privacy #355

Closed
mainrs opened this issue Oct 5, 2019 · 8 comments · Fixed by #915
Closed

Add debug CLI config that prevents logging from taking out values for privacy #355

mainrs opened this issue Oct 5, 2019 · 8 comments · Fixed by #915
Assignees
Labels
enhancement A new feature that would improve Spotifyd good first issue An easy to implement issue, good for first time contributors or people new to open source
Milestone

Comments

@mainrs
Copy link
Member

mainrs commented Oct 5, 2019

This would be a nice to have feature to make debugging BadCredentials during the login step of the daemon easier to debug. It would allow people to see their password and username used within the configuration.

How to implement:

  • Add a boolean flag (name is up for discussion) to the SharedConfigValues struct.
  • Check for the flag in this method and change the values accordingly.
@mainrs mainrs added enhancement A new feature that would improve Spotifyd good first issue An easy to implement issue, good for first time contributors or people new to open source labels Oct 5, 2019
@mainrs mainrs added this to the v0.2.19 milestone Oct 7, 2019
@olavbm
Copy link

olavbm commented Oct 18, 2019

I'd like to contribute here.

@mainrs
Copy link
Member Author

mainrs commented Oct 18, 2019

Someone created a PR linked above (#360). We came to the conclusion that it would be better to move our Debug implementation one scope up in the chain. See #360 (comment) for more information.

I am not sure how much experience you have with Rust. This might be a harder issue to resolve than other issues I marked with “good first issue”. Feel free to give it a try though!

You can open a new PR after you implemented it. Make sure to link back to this issue and the PR I mentioned above.

For now I’ll assign you to the issue. It you have problems implementing it let me know and I’ll tackle this one down.

@mainrs
Copy link
Member Author

mainrs commented Oct 18, 2019

Correction on my part. The Debug implementation doesn’t need to be moved. CliConfig just needs one too that calls the one of SharedConfigValues.

I am typing this up from my memory. So if you think I am talking gibberish, please let me know :)
I just don’t want you people wasting time on an issue that might actually not work as I imagine it right now. That’s why I wrote in #360 that I’ll take a closer look myself :)

@mainrs mainrs modified the milestones: v0.2.20, v0.2.21 Dec 17, 2019
@mainrs mainrs modified the milestones: v0.2.21, v0.2.25 Feb 7, 2020
@slondr slondr modified the milestones: v0.2.25, v0.2.26, v0.3.1 Jan 12, 2021
@Icelk
Copy link
Contributor

Icelk commented Feb 19, 2021

Hi @sirwindfield ! Can I help out with this issue?
Should I do as you stated in the first comment?

@robinvd
Copy link
Contributor

robinvd commented Mar 2, 2021

@IsELK yes that should do it! If you need any help you can ping me here or in a pr

@jackvstrickland
Copy link

Hi @sirwindfield ! Can I help out with this issue?
Should I do as you stated in the first comment?

have you started yet? If not, I wouldn't mind taking a crack at it

@mainrs mainrs assigned Icelk and unassigned olavbm Mar 10, 2021
@mainrs
Copy link
Member Author

mainrs commented Mar 10, 2021

I'm not actively developing anymore, just keeping a watchful eye around here ;) I've assigned @IsELK as they offered their help first. If we do not receive any updates you can gladly take over @jackvstrickland.

In the meantime, there are other issues that need help :)

@Icelk
Copy link
Contributor

Icelk commented Mar 10, 2021

Hi @jackvstrickland

have you started yet? If not, I wouldn't mind taking a crack at it

I am. It will be done this week, probably on Friday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature that would improve Spotifyd good first issue An easy to implement issue, good for first time contributors or people new to open source
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants