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

Fix the config/tokens endpoint #113

Merged
merged 2 commits into from
Sep 8, 2017

Conversation

stuartlangridge
Copy link
Contributor

Fix the config/tokens endpoint, which does not work from bin/cc as it currently stands.

bin/cc sends a list of tokens as a simple JSON string, colon-separated, by PUT. This gets rejected by strict body-parser JSON parsing (which expects JSON to be an array or an object), and also by config.js which expects config/tokens to be addressed as POST.
Add a GET verb to the /config/tokens endpoint to list the tokens that have already been added.

(Fixes microsoft/ghcrawler-cli#8)

… currently stands.

bin/cc sends a list of tokens as a simple JSON string, colon-separated. This gets rejected by strict body-parser JSON parsing (which expects JSON to be an array or an object).
Add a GET verb to the /config/tokens endpoint to list the tokens that have already been added.
@msftclas
Copy link

msftclas commented Sep 8, 2017

@stuartlangridge,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@msftclas
Copy link

msftclas commented Sep 8, 2017

@stuartlangridge, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

stuartlangridge added a commit to stuartlangridge/ghcrawler-cli that referenced this pull request Sep 8, 2017
@jeffmcaffer
Copy link
Contributor

Thanks for the pull request. As the tokens are key secrets and typically have very significant permissions, I'm wary of exposing them via a REST API. Can you outline the scenario where that functionality is needed?

@stuartlangridge
Copy link
Contributor Author

Fair point on the exposure. The scenario where it's needed is this: me asking "have I added this token? which ones have I added already?". Not being able to find this out at all is quite frustrating, especially when you're experimenting with the token functionality :) If you think it's too risky then dropping it from the REST API is reasonable, but having some way to list the tokens that the crawler already knows about would be much appreciated!

@jeffmcaffer
Copy link
Contributor

got it. What we do in the logs is output part of the token (see https://github.com/Microsoft/ghcrawler/blob/develop/providers/fetcher/githubFetcher.js#L51). That is generally enough for debugging purposes. Would that work for you?

@stuartlangridge
Copy link
Contributor Author

It would! That's a good thought, indeed.

@stuartlangridge
Copy link
Contributor Author

(pushed a new commit which ellipsises tokens to 8 characters)

@jeffmcaffer jeffmcaffer merged commit 4047492 into microsoft:develop Sep 8, 2017
@jeffmcaffer
Copy link
Contributor

Thanks for the changes @stuartlangridge

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.

3 participants