Skip to content

Conversation

@legnoh
Copy link
Contributor

@legnoh legnoh commented Oct 30, 2025

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the master branch.
  • I used gofmt for formatting the code before submitting the pull request.

This pull request adds new functionality to manage access tokens more effectively, including retrieving and revoking tokens by ID, as well as filtering and listing tokens. It introduces new methods in the access token service and manager, updates the documentation, and adds comprehensive tests for these features.

New Access Token Management Features:

Documentation Updates:

  • Expanded the README.md to document:
    • How to get access tokens (with filters)
    • How to get a token by ID
    • How to revoke a token by ID [1] [2]

Testing Enhancements:

  • Added tests for the new access token methods:
    • testGetTokens
    • testGetTokenByID
    • testRevokeTokenByID [1] [2]

These changes make it easier to programmatically manage access tokens, improving both usability and test coverage.

@ehl-jf ehl-jf added the safe to test Approve running integration tests on a pull request label Oct 30, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Oct 30, 2025
@legnoh legnoh marked this pull request as draft October 30, 2025 03:55
@legnoh legnoh changed the title Add get tokens by Add GetTokens, GetTokensByID, RevokeTokenByID methods Oct 30, 2025
@ehl-jf ehl-jf added the safe to test Approve running integration tests on a pull request label Oct 30, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Oct 30, 2025
@ehl-jf ehl-jf added the safe to test Approve running integration tests on a pull request label Oct 30, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Oct 30, 2025
@ehl-jf ehl-jf added the safe to test Approve running integration tests on a pull request label Oct 30, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Oct 30, 2025
@legnoh legnoh changed the title Add GetTokens, GetTokensByID, RevokeTokenByID methods feat: Add GetTokens, GetTokensByID, RevokeTokenByID methods Oct 30, 2025
@legnoh legnoh marked this pull request as ready for review October 30, 2025 04:16
README.md Outdated
Username: "admin", // Filter by username
Refreshable: utils.Pointer(true), // Filter by refreshable status
TokenId: "token-id", // Filter by specific token ID
OrderBy: "issued_at", // Order by field (issued_at, expires_at, etc.)

Choose a reason for hiding this comment

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

// Order by field (created,token_id,owner,subject,expiry)
https://jfrog.com/help/r/jfrog-rest-apis/get-tokens

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right — the information was inaccurate. I'll correct it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the correction!

}
if params.LastUsed != nil {
queryParams.Add("last_used", strconv.FormatInt(*params.LastUsed, 10))
}

Choose a reason for hiding this comment

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

last_used should be removed as it is not supported by the backend

Copy link
Contributor Author

@legnoh legnoh Dec 12, 2025

Choose a reason for hiding this comment

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

When I look at the document below, it suggests in a Note that the last_used parameter is available under certain conditions — is that not correct?
https://jfrog.com/help/r/jfrog-rest-apis/get-tokens

Choose a reason for hiding this comment

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

The Note holds true for the response payload. But its not supported as query param

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. I've made the correction, so please take a look.

if err != nil {
return err
}
return errorutils.CheckResponseStatusWithBody(resp, body, http.StatusOK)
Copy link

Choose a reason for hiding this comment

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

Add http.StatusNoContent as well.API will return 204 incase if token doesn't exist or already revoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it!

attiasas and others added 6 commits December 17, 2025 18:25
* Add retry to POST build scan trigger if needed

* retry err as suffix

* build name can be not alphabet

* fix regex number

* use retry

* add debug log

* fix tests
* Xray Remediation Service

* add remediation service to xray manager

* rename facade method

* start adding tests for new API

* switch to new API

* start fix tests

* fix tests

* fix tests

* fix tests

* fix tests

* fix tests

* fix tests

* fix tests

* add content type

* fix attribs in resp struct

* CR changes
* Removed repo field in AQL query

* Updated test case
dortam888 and others added 15 commits December 17, 2025 18:25
…#1265)

* Fix an issue that evidence encoded url twice resulting in a wrong url

* Add evidence client mock in order to make evidence APIs more accurate in evidence tests
* Xray Get Violations API

* change upload log to debug

* fix log remove file

* add NewWatchRepositoryByName

* fix create watch body

* format

* CR changes

* add LineNumber to API
* Fix application details resolution from api

* Add new fields for criticality and maturity to README
* added git integration post request

* added function to XscService interface

* added SendGitIntegrationEvent to xsc inner service

* after cr
* Fix Remediation after API breaking change

* add more changes to the API

* fix test
* malicious-code-scanner

* malicious-code-scanner
* updated CC structs and comparison file

* removing DisplayFixableOnly and MinSeverityToDisplay

* setting back ConfigProfileNewSchemaMinXrayVersion
@github-actions
Copy link
Contributor

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


@legnoh
Copy link
Contributor Author

legnoh commented Dec 17, 2025

The upstream merge ended up in an odd state, so I'll recreate the PR.
Sorry for the trouble....

@legnoh
Copy link
Contributor Author

legnoh commented Dec 17, 2025

@0x01F4 @abhisheksaxena29 I recreated the PR(#1282) to ensure the diff is applied correctly.
I would appreciate it if you could review it again 🙇

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.