Skip to content

RTDEV-61004 - Take the evidence created by user from the token#9

Closed
selalererjfrog wants to merge 1 commit intomainfrom
feature/RTDEV-61004
Closed

RTDEV-61004 - Take the evidence created by user from the token#9
selalererjfrog wants to merge 1 commit intomainfrom
feature/RTDEV-61004

Conversation

@selalererjfrog
Copy link
Copy Markdown

Pull Request Template

Description

Fixed security issue where user in the CLI config file is displayed as the user that created the evidence regardless of who is logged in.

Testing

  • [v] Tests added/updated
  • [v] All tests pass locally

Checklist

  • [v] PR description is clear and concise, and it includes the proposed solution/fix
  • [v] Code follows project style guidelines
  • [v] Documentation updated (if applicable)

func (c *createEvidenceBase) getUser() string {
user := auth.ExtractUsernameFromAccessToken(c.serverDetails.AccessToken)
if user == "" {
user = EvdDefaultUser
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should first fall-back the user from serverDetails and only after to the default one

	user := c.serverDetails.User
	if user == "" {
		user = EvdDefaultUser
	}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The ticket complains about us taking the user from the config even though it is not an authenticated user.
Evidence can't be created anyway with user/password authentication, so the only valid place to take the user is from the token

assert.Equal(t, "abc123", st.Subject[0].Digest.Sha256)
assert.Equal(t, "hello", st.Markdown)
assert.Equal(t, "alice", st.CreatedBy)
assert.Equal(t, "moshe", st.CreatedBy)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what's wrong with alice? :))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's not what I have in this token :-)

@github-actions
Copy link
Copy Markdown
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


@selalererjfrog
Copy link
Copy Markdown
Author

After discussion with Yevdo, this might not be a good place to solve this security concern

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Security-related fixes or improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants