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

Add username field to secure repos #3116

Merged
merged 2 commits into from
Jun 15, 2020
Merged

Conversation

ebuckle
Copy link
Contributor

@ebuckle ebuckle commented Jun 10, 2020

Signed-off-by: Edward Buckle edward.buckle0@gmail.com

What type of PR is this ?

  • Bug fix
  • Enhancement

What does this PR do ?

For secure template repos, the template object will now contain a field for the user's github username, if available. If not available, the authorization object will be empty:
Screen Shot 2020-06-10 at 14 52 48
Screen Shot 2020-06-10 at 14 53 57

Which issue(s) does this PR fix ?

#3101

Link to the Codewind repository issue(s) this PR fixes or references:

Does this PR require a documentation change ?

Yes.

Any special notes for your reviewer ?

None.

@@ -323,6 +323,9 @@ async function constructRepositoryObject(url, description, name, isRepoProtected
repository = await fetchRepositoryDetails(repository, gitCredentials);
if (isRepoProtected !== undefined) {
repository.protected = isRepoProtected;
Copy link
Contributor

Choose a reason for hiding this comment

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

fyi protected means "this repository should not be deleted". (It doesn't mean "this repository is secure / requires authentication")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks! Do we have an alternative? Or do we only need git user/password on authenticated repos?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea we only need to add the authenticated field to the template repo object if the repo is authenticated / requires auth.

So your 'trigger' for adding the field can be just if there are gitCredentials. (Maybe check that if the gitCredentials are invalid we will error before reaching this point)

Copy link
Contributor

@rwalle61 rwalle61 left a comment

Choose a reason for hiding this comment

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

Btw, how does this code make the response body include authentication: {} when gitCredentials is the personalAccessToken?

It'd be good to see some tests with this, and an update to our openapi.yaml :)

Signed-off-by: Edward Buckle <edward.buckle0@gmail.com>
Signed-off-by: Edward Buckle <edward.buckle0@gmail.com>
@ebuckle ebuckle requested a review from elsony as a code owner June 10, 2020 15:49
Copy link
Contributor

@hhellyer hhellyer left a comment

Choose a reason for hiding this comment

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

LGTM - Will pass the username field through if it is set.

@hhellyer hhellyer merged commit 6935e02 into eclipse-archived:master Jun 15, 2020
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