-
Notifications
You must be signed in to change notification settings - Fork 15
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
Feature - Mapped Collection Token Support #1334
Feature - Mapped Collection Token Support #1334
Conversation
…UserAccessTokenReply to support finding correct tokens
…& DatabaseAPI.hpp Use updated names and types. ClientWorker.cpp Update comments.
…oken match and missing match cases
…ir/list to test out changes.
…es on covered cases; add notes on code
…lientWorker.cpp Add refresh token try/catch to send through flow if refresh fails
…irect to error when needs_consent
…duced for handling logic for validating params when getting tokens and building GetAccessToken responses. user_router.js Replace logic with calls to new lib
…n. user_router.js Remove unnecessary comment.
…ken document required fields. user_token.test.js Add preliminary unit tests for static methods.
… naming to match convention.
… Add simple tests for get/token endpoint. user_token.js Fix naming for token document scopes field. user_token.test.js Changes for scope field bugfix.
…, add note about token type.
…API.cpp Remove TODOs. SDMS_Auth.proto Remove unused field.
…DatabaseAPI.cpp Add note on possibly missing field. user_token.test.js Update tests to check for new expected values.
…5-foxx-UserGetAccessRequest-mapped-collection
…nformation, JSDoc. tasks.js Include context required in call to getAccessToken, add comment on refresh. TaskWorker.cpp Identify where changes will be reflected.
optional string collection_id = 6; // Globus Collection ID | ||
optional string collection_type = 7; // Globus Collection Type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great
const get_globus_token_model_collection = "a1067823-2598-4481-be95-712794ddd9e8"; | ||
const get_globus_token_model_collection_id = "globus_coll/" + get_globus_token_model_collection; | ||
const globus_collection_token_collection = "126a23d4-45ed-49fb-bde2-76b5f44d20d1"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain these? I'm getting a bit cross eyed reading them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between the model and collection and token_collection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming is hard.
get_globus_token_model_user
and
get_globus_token_model_collection
are for the Globus Token model tests, specifically around gets/reads.
where
user_collection_token_user
and
globus_collection_token_collection
are meant for the full user-token relationship tests in UserToken
If you can suggest better names, I'm all ears.
if (a_request.has_collection_id()) { | ||
payload["collection_id"] = a_request.collection_id(); | ||
} | ||
|
||
if (a_request.has_collection_type()) { | ||
payload["collection_type"] = a_request.collection_type(); | ||
} | ||
|
||
string body = payload.dump(-1, ' ', true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, the JSON object help simplify this.
|
||
string body = payload.dump(-1, ' ', true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, it could be useful to add comments in both requests where this is used to incidate that this is compacting the json before sending. It's not immediately clear from looking at the code that this is what is happening.
me.m_glob.refreshAccessToken(ref_tok, acc_tok, expires_in); | ||
me.m_db.userSetAccessToken(acc_tok, expires_in, ref_tok, log_context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if these fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this was the default behavior anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe both refresh
and ...Set...
throw errors if they fail, so those errors get propagated up. I put a specific try/catch around the new case so that we also see in logs that the error is related to a mapped collection, in case that helps to narrow down debugging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice
.toArray(); | ||
|
||
if (token_matches.length > 0) { | ||
if (token_matches.length > 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonder if we should have some gating logic to prevent adding a token if a user already has a token associated with a collection_id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm not sure there will ever be a second, for example, transfer token on an collection for a given user. I just saw that the way I am retrieving it produces an array and thought that should be explicit in code.
#map_entry_to_oauth_token() { | ||
let oauth_token = new DataFedOAuthToken(); | ||
Object.keys(DataFedOAuthToken).map((key) => { | ||
console.log("mapping key ", key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.log("mapping key ", key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, definitely need to clean that up. Good catch.
"use strict"; | ||
|
||
const support = require("../support"); | ||
const { DataFedOAuthToken } = require("./DataFedOAuthToken"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You got it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I accidentally created a circular dependency, and apparently JS/Node will just roll with it without error, but substitute undefined
* @param {number} [token_document.expiration] - Expiration time of access token | ||
* @param {g_lib.AccessTokenType | number} [token_document.type] - Access token type, present when retrieving a collection token | ||
* @param {string} [token_document.dependent_scopes] - Access token scopes, present when retrieving a collection token | ||
* @param {DataFedOAuthToken} token_document - Database document holding relevant token info |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is any of this exported (mock data), or it directly in the live session?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of it is exported, these are inserted into the DB on tests
"${FOXX_PREFIX}foxx" script -u "${local_DATABASE_USER}" \ | ||
--server "tcp://${local_DATAFED_DATABASE_HOST}:8529" \ | ||
-p "${PATH_TO_PASSWD_FILE}" \ | ||
--database "${local_DATABASE_NAME}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess we're doing it live
collectionId: a_req.query.collection_id, | ||
collectionType: a_req.query.collection_type, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more session? Do we need that anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It ended up being easiest for me to implement like this, but we're using session for set
on the tokens still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can, and likely should, discuss whether or how to use sessions moving forward.
@@ -1953,6 +1957,16 @@ app.get("/ui/ep/dir/list", (a_req, a_resp) => { | |||
}); | |||
|
|||
req.end(); | |||
}; | |||
|
|||
sendMessage("UserGetAccessTokenRequest", { ...message_data }, a_req, a_resp, function (reply) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you context dump me on this request pls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UserGetAccessTokenRequest
protobuf message is passed here:
DataFed/core/server/ClientWorker.cpp
Lines 1356 to 1358 in 606f8ac
std::unique_ptr<IMessage> ClientWorker::procUserGetAccessTokenRequest( | |
const std::string &a_uid, std::unique_ptr<IMessage> &&msg_request, | |
LogContext log_context) { |
Which is then sent here on the API:
DataFed/core/database/foxx/api/user_router.js
Lines 699 to 706 in 8ea7df6
router | |
.get("/token/get", function (req, res) { | |
try { | |
const collection_token = UserToken.validateRequestParams(req.queryParams); | |
// TODO: collection type determines logic when mapped vs HA | |
const { collection_id, collection_type } = req.queryParams; | |
var user; |
The API function uses the logic in this PR to find the token, and if it doesn't find an appropriate token when provided both collection_id and collection_type (or any other failure condition) it will flag needs_consent
Control is then passed back to the core service where this logic handles the rest:
DataFed/core/server/ClientWorker.cpp
Lines 1384 to 1403 in 606f8ac
if (needs_consent) { | |
// short circuit to reply | |
} else if (expires_in < 300) { | |
DL_INFO(log_context, "Refreshing access token for " << a_uid); | |
if (token_type == AccessTokenType::GLOBUS_DEFAULT) { | |
m_globus_api.refreshAccessToken(ref_tok, acc_tok, expires_in); | |
m_db_client.userSetAccessToken(acc_tok, expires_in, ref_tok, log_context); | |
} else { | |
try { | |
m_globus_api.refreshAccessToken(ref_tok, acc_tok, expires_in); | |
m_db_client.userSetAccessToken( | |
acc_tok, expires_in, ref_tok, (AccessTokenType)token_type, | |
collection_id + "|" + scopes, log_context); | |
} catch (TraceException &e) { // NOTE: assumes refresh failed (invalid or | |
// failure). new token fetched will upsert | |
// and overwrite old values on database | |
needs_consent = true; | |
} | |
} | |
} |
And will pass the needs_consent
flag back along, and will even set it if a refresh is triggered and fails.
}); | ||
|
||
if (data.code) { | ||
if (data.needs_consent || data.code) { | ||
// TODO: needs consent flag only works first time, if base token has consent it will no longer work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the base token has consent
I don't know if needs_consent
exists on the response from epDirList
via Globus. Did we add that in via our own logic where we check if the user has consent for x collection_id
?
{
"code": "ConsentRequired",
"message": "Missing required data_access consent",
"request_id": "TmSqBWfVN",
"required_scopes": [
"urn:globus:auth:scope:transfer.api.globus.org:all[*https://auth.globus.org/scopes/6e9de696-6a3b-4dfe-8a8b-e4faeaccb2be/data_access]"
],
"resource": "/operation/endpoint/6e9de696-6a3b-4dfe-8a8b-e4faeaccb2be/ls"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
This was created with Devel
Question
Is your work expected to lead to this? This is before your changes (on devel)
Steps to Replicate
- Right-click
- Hover over New
- Left-click Data Record
- Fill out Title
- Select paper icon on Source field
- Input mapped collection by name in Source Path
- If found or automatched, will appear in Source Endpoint dropdown
- Select endpoint from dropdown
- Select Browse button
- See
Consent required
- Close modal
- Select Browse button
- See
Error: Token is not active
Artifacts
Payload Response
{
"code": "AuthenticationFailed",
"message": "Token is not active",
"request_id": "7hgsj0paJ",
"resource": "/operation/endpoint/6e9de696-6a3b-4dfe-8a8b-e4faeaccb2be/ls"
}
Visual error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For your initial comment, added it here: https://github.com/ORNL/DataFed/pull/1334/files#diff-fd6b6aa4834b6e169808365a711db1d97195030c67f9ca27ae9fc156052ce93cR1949
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the big drawback I was finding here is that if we somehow lose the stored token (like a database rollback) then when a user tries to log in again, they will not use the given consent flow to store a new token, as I would desire, but instead the response from Globus will play out exactly as if they had the appropriate token (so basically a stored state disagreement between DataFed and Globus).
The only way I can see this edge case being corrected without input is:
A user has revoked consent, and received an explicit consent token (like we try to store)
What to do for a user experiencing this:
- Have the user navigate to https://app.globus.org/settings/consents
- Have them click the highlighted "Manage Your Consents" button
- Have them scroll to the appropriate DataFed GCS client (name will vary) and click the dropdown arrow:
- Have them click the garbage bin icon next to the problem collection:
- Once the collection is no longer on that dropdown list, retry flow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is likely an API call we can lead them to or implement on our side, as well, but this is what I can show for now.
const { collection_id, collection_type } = a_req.query; | ||
par.collectionId = collection_id; | ||
par.collectionType = collection_type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can reduce this to two lines with this
par.collectionId = a_req.query.collection_id;
par.collectionType = a_req.query.collection_type;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens here if collection_id is none or collection_type is none?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're uninitialized on the protobuf message, DataPutRequest
which then is sent here:
DataFed/core/server/ClientWorker.cpp
Line 750 in 606f8ac
ClientWorker::procDataPutRequest(const std::string &a_uid, |
Which goes here:
DataFed/core/server/DatabaseAPI.cpp
Line 3209 in a83fa6a
if (a_request.has_collection_id()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I think the destructuring into assignments is cleaner code than assigning from a nested field in an object.
collectionId: a_req.session.collection_id, | ||
collectionType: a_req.session.collection_type, | ||
collectionId: a_req.query.collection_id, | ||
collectionType: a_req.query.collection_type, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the thought behind storing this in a session and then not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally we were storing it via express-session (cause easier to grab via context). With the collection_id
being unavailable, that's where the problem starts. Easier to be direct and get it via request handling IIRC
CC: @t-ramz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aaron's right on point with it, way easier to pass it as a request. Especially since Aaron's new UI code makes it so much cleaner to read.
const token_doc = new UserToken({ | ||
user_id: a_task.client, | ||
globus_collection_id: state.collection_info.collection_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens here if collection_info is {}?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will pass in undefined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it will pass in undefined, and the UserToken
object will check specifically for undefined here: https://github.com/ORNL/DataFed/blob/feature-DAPS-1214-data-put-get-mapped-collections/core/database/foxx/api/lib/user_token.js#L34
So if it somehow comes through null, we should get an error thrown by the inside GlobusCollectionModel constructor. Not sure if that's the best way to go about it, but it catches a fair amount of scenarios.
throw [g_lib.ERR_NOT_FOUND, "Specified user does not exist: " + kwargs]; | ||
} | ||
this.user = this.#user_model.get(); | ||
if (typeof globus_collection_id !== "undefined") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so here you are handling undefined, nice.
User cases demoNew Data RecordGuest collectionScreen.Recording.2025-03-05.094518.mp4Mapped collectionScreen.Recording.2025-03-05.095114.mp4Update Data RecordGuest collectionScreen.Recording.2025-03-05.095705.mp4Mapped collection (consent granted in previous step)Screen.Recording.2025-03-05.095901.mp4Mapped collection (consent flow)Screen.Recording.2025-03-05.103756.mp4Download Data RecordGuest collectionpt 1 Screen.Recording.2025-03-05.102040.mp4pt 2 Screen.Recording.2025-03-05.102433.mp4Mapped collection (consent already granted)pt 1 Screen.Recording.2025-03-05.102736.mp4pt 2 Screen.Recording.2025-03-05.102956.mp4Mapped collection (consent flow)pt 1 Screen.Recording.2025-03-05.103304.mp4pt 2 Screen.Recording.2025-03-05.103522.mp4 |
if (this.exists()) { | ||
// TODO: abstract database interactions | ||
let query = { _id: this.#user_id }; | ||
if (this.#user_key) { | ||
query = { _key: this.#user_key }; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these lines necessary if you are already checking if (this.exists())
looks like it does the same thing to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's changing the query in particular. Since I wanted the ability to search by ID or key, I switch query accordingly.
const { collection_id, collection_type } = a_req.query; | ||
par.collectionId = collection_id; | ||
par.collectionType = collection_type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I think the destructuring into assignments is cleaner code than assigning from a nested field in an object.
* Merge hotfixes into devel (#1326) * [DAPS-DEPS] Update cpp-py-formatter to 0.3.0 * [DAPS-DEPS] Update cpp-py-formatter to 0.3.1 * [DAPS-DEPS] Update ver, add push * updated versions for release * Update project.rst * Update format-check.yml * Pull in version numbers appropriately * Avoid build cache * Aggressive prune * Trigger rebuild * generate_repo_config.sh Accept env variable for datafed server from repo service instead of hard coding * Allow core config thread options to be read in from env * Add changelog comment * Adjust deployment version number * Fix Version month * Update CHANGELOG.md Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --------- Co-authored-by: Aaron Perez <perezam@ornl.gov> Co-authored-by: Blake Nedved <nedvedba@ornl.gov> Co-authored-by: Joshua S Brown <joshbro42867@yahoo.com> Co-authored-by: JoshuaSBrown <brownjs@ornl.gov> Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> Co-authored-by: nedvedba <145805866+nedvedba@users.noreply.github.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> * Feature DAPS 1215 foxx UserGetAccessTokenRequest mapped collection support (#1284) * Begin prototyping required code for mapped collection token retrieval. * SDMS_Auth.proto Add optional fields to UserGetAccessTokenRequest and UserAccessTokenReply to support finding correct tokens * user_router.js Add baseline query params and checks. DatabaseAPI.cpp & DatabaseAPI.hpp Use updated names and types. ClientWorker.cpp Update comments. * user_router.js Add logic for finding token based on collection; add token match and missing match cases * Return necessary data from Database to refresh tokens. Mock data on dir/list to test out changes. * user_router.js Add check for existence of Globus collection * user_router.js Remove existence check, move to filter method; Add notes on covered cases; add notes on code * datafed-ws.js Temporary handling of need consent response. * user_router.js Add queryParam validation for new params via joilib. ClientWorker.cpp Add refresh token try/catch to send through flow if refresh fails * datafed-ws.js Use correct converted casing for protobuf messages; redirect to error when needs_consent * datafed-ws.js No redirect, show error directly in list. * datafed-ws.js Early return to prevent further requests * support.js Update AccessTokenType enum. user_token.js New class introduced for handling logic for validating params when getting tokens and building GetAccessToken responses. user_router.js Replace logic with calls to new lib * user_token.js Add jsdoc definition for return type and format function. user_router.js Remove unnecessary comment. * user_token.js More jsdoc definition; Add validation for collection token document required fields. user_token.test.js Add preliminary unit tests for static methods. * CMakeLists.txt Add new unit tests to build. user_token.test.js Modify naming to match convention. * CMakeLists.txt Fix variable get. user_token.js Fix export. * user_fixture.js Add new user for get/token tests. user_router.test.js Add simple tests for get/token endpoint. user_token.js Fix naming for token document scopes field. user_token.test.js Changes for scope field bugfix. * user_router.js Formatting; Throw error if more than one token matches, add note about token type. * ClientWorker.cpp Address some TODOs and extraneous comments. DatabaseAPI.cpp Remove TODOs. SDMS_Auth.proto Remove unused field. * user_token.js Update formatUserToken to always return object values. DatabaseAPI.cpp Add note on possibly missing field. user_token.test.js Update tests to check for new expected values. * user_router.js Clean up some comments for clarity. datafed-ws.js Formatting. * ClientWorker.cpp Rewrite comment regarding mapped token refresh. * user_token.js Formatting. * CHANGELOG.md update for feature * user_router.js Re-introduce accidentally removed `globus_collection` object. * CMakeLists.txt Remove comment about necessity * Update token lookup logic to use `byExample`, add test case for missing Globus collection. Add globus_coll fixture. * SDMS_Auth.proto Change UserAccessTokenReply needs_consent to optional field. * user_router.js Update token/get to default to start consent flow when collection tokens are present. user_router.test.js Update test from error to needs_consent. * Formatting changes. --------- Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> * [DLT-1110] Implement Consent Required Action (3/4 & 4/4) (#1242) * [DLT-1110] Refactor browse into component and use CSS * [DAPS-1110] Update * [DAPS-1110] Update web server * [DAPS-1110] add refactors * [DAPS-1110] Prettier * [DAPS-1110] Refactor tests * [DAPS-1110] Remove .. in ep ls, remove logs * [DAPS-1110] Add sinon, remove dead code * [DAPS-1110] Address transfer start button bug * [DAPS-1110] eslint, update api.test.js to reflect usage * [DAPS-1110] format * [DAPS-1110] Correct bug * JSON parse error fix (#1328) * DatabaseAPI.cpp Pull in json dependency, use json object to serialize request payload * Replace json serialization in more locations where parseSearchRequest is being called. * Add comments around locations that will need json serialization * Convert missed body append to serialization format * typo * Refactoring DatabaseAPI.cpp to incorporate json serialization * More serialization work * Pull in changes from 1214 for dataPut * Bring in changes from 1214 for dataGet * DatabaseAPI.cpp Fix error with missing curly brace. Some formatting that is likely wrong. Serialize up to original point. Need to validate. * Add missing scope resolution * More typos * More typos. Scoping * Remove non-existant vars * declare body * Formatting * Formatting and verification of updates * Replace most places * Formatting * Remove redeclaration * DatabaseAPI.cpp Remove some comments. * DatabaseAPI.cpp Add missing push_back * Adding braces * Prevent Double Escaping string, by using nlohmann json parse to read in value. * remove escapeJSON calls, redundant with nlohmann json dump * Allow parsing for metric data to be run in parallel for now, output messages when old serialization is used. * Ensure ASCII setting. * Upgrade debug to warning. --------- Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> Co-authored-by: Austin Hampton <amh107@latech.edu> Co-authored-by: Joshua S Brown <joshbro42867@yahoo.com> * Feature - Mapped Collection Token Support (#1334) * Begin prototyping required code for mapped collection token retrieval. * SDMS_Auth.proto Add optional fields to UserGetAccessTokenRequest and UserAccessTokenReply to support finding correct tokens * user_router.js Add baseline query params and checks. DatabaseAPI.cpp & DatabaseAPI.hpp Use updated names and types. ClientWorker.cpp Update comments. * user_router.js Add logic for finding token based on collection; add token match and missing match cases * Return necessary data from Database to refresh tokens. Mock data on dir/list to test out changes. * user_router.js Add check for existence of Globus collection * user_router.js Remove existence check, move to filter method; Add notes on covered cases; add notes on code * datafed-ws.js Temporary handling of need consent response. * user_router.js Add queryParam validation for new params via joilib. ClientWorker.cpp Add refresh token try/catch to send through flow if refresh fails * datafed-ws.js Use correct converted casing for protobuf messages; redirect to error when needs_consent * datafed-ws.js No redirect, show error directly in list. * datafed-ws.js Early return to prevent further requests * support.js Update AccessTokenType enum. user_token.js New class introduced for handling logic for validating params when getting tokens and building GetAccessToken responses. user_router.js Replace logic with calls to new lib * user_token.js Add jsdoc definition for return type and format function. user_router.js Remove unnecessary comment. * user_token.js More jsdoc definition; Add validation for collection token document required fields. user_token.test.js Add preliminary unit tests for static methods. * CMakeLists.txt Add new unit tests to build. user_token.test.js Modify naming to match convention. * CMakeLists.txt Fix variable get. user_token.js Fix export. * user_fixture.js Add new user for get/token tests. user_router.test.js Add simple tests for get/token endpoint. user_token.js Fix naming for token document scopes field. user_token.test.js Changes for scope field bugfix. * user_router.js Formatting; Throw error if more than one token matches, add note about token type. * ClientWorker.cpp Address some TODOs and extraneous comments. DatabaseAPI.cpp Remove TODOs. SDMS_Auth.proto Remove unused field. * user_token.js Update formatUserToken to always return object values. DatabaseAPI.cpp Add note on possibly missing field. user_token.test.js Update tests to check for new expected values. * user_router.js Clean up some comments for clarity. datafed-ws.js Formatting. * ClientWorker.cpp Rewrite comment regarding mapped token refresh. * user_token.js Formatting. * CHANGELOG.md update for feature * user_router.js Re-introduce accidentally removed `globus_collection` object. * CMakeLists.txt Remove comment about necessity * Add collection_type and collection_id to dat/put task state * support.js Update getAccessToken method to work based on collection information, JSDoc. tasks.js Include context required in call to getAccessToken, add comment on refresh. TaskWorker.cpp Identify where changes will be reflected. * Update token lookup logic to use `byExample`, add test case for missing Globus collection. Add globus_coll fixture. * SDMS_Auth.proto Change UserAccessTokenReply needs_consent to optional field. * user_router.js Update token/get to default to start consent flow when collection tokens are present. user_router.test.js Update test from error to needs_consent. * Formatting changes. * SDMS_Auth.proto Add new fields to DataGetRequest * Begin refactor of token decision logic. * Address bugs in fetching and mapping data, make unit tests pass. * globus_token.js Match other models for get * tasks.js Pull in new logic for determining tokens, replace tokens; Needs to be verified. user_token.js Add new format function for compatibility with transfer doc replacement. * DatabaseAPI.cpp Sane serialization of json body for taskInitDataPut. TaskWorker.cpp Comments around work items. * DatabaseAPI.cpp Sane serialization of json body for taskInitDataGet. * ClientWorker.cpp Stub out required changes for consent flow when creating tasks * ClientWorker.cpp More thorough consent flow for procDataPutRequest, stub on needs_consent * tasks.js Throw error when initializing data put * tasks.js Format error throwing. ClientWorker.cpp comment out potentially unnecessary code. * user_token.js Add exists method to abstract check. tasks.js User exists method when checking token. * user_token.js JSDoc typing * tasks.js Add more required fields to taskRunDataPut for refresh in TaskWorker. * tasks.js Add collection_id to params in taskRunDataPut for refresh in TaskWorker. TaskWorker.cpp Refresh conditionals * DatabaseAPI.cpp Update merged devel changes to address some leftover code. * DatabaseAPI.cpp Add collection specification to get request. * DatabaseAPI.cpp whitespace. ClientWorker.cpp Clean up unused code. user_router.js Switch to checking token existence through standard API. * user_token.js Add comment about type return. data_router.js Do validations at router level for user collection token relationship. tasks.js Remove validation from logic level; make dataGet mirror dataPut; Updates to naming and params structure. * ClientWorker.cpp Revert to minimize diff. TaskWorker.cpp Remove unnecessary TODO; move set client for refresh in transfer tasks. * models/ JSDocs, formatting, make token fetches follow defined model. user_token.js JSDocs and typing. * user_token.js Cover case where collection does not exist. * Small changes for UI detection of collection type * fix epview call for UI changes, add list view change to react appropriately to needing consent according to current code * extract ep view logic to function for either path to utilize * small changes and notes for UI updates * UI search based on path, not ID. camelCasing * TaskWorker.cpp small fix to only enforce requirement of additional refresh fields when token is transfer token. * add params for dat/get on UI * task.js Fix bug where client is incorrectly referenced. * Add user model tests. Extract DataFedOAuthToken.js from user_token.js for reusability. * Add globus collection model tests. * Add globus token tests. Add fixtures to support globus token testing. Make all model properties static for iteration purposes. Utilize constructors for typing. * Add user token tests. Make non-existent user throw an error when building user tokens. * Change error to be not found * Formatting changes * Remove unused changes in support.js getAccessToken * Changelog update, update comments, remove log. --------- Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> * Add small fix for storeCollectionId (#1341) * Add small fix for storeCollectionId * move todo comment --------- Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> --------- Co-authored-by: Anthony <64566330+t-ramz@users.noreply.github.com> Co-authored-by: Aaron Perez <perezam@ornl.gov> Co-authored-by: Joshua S Brown <joshbro42867@yahoo.com> Co-authored-by: JoshuaSBrown <brownjs@ornl.gov> Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> Co-authored-by: Aaron Perez <aperez0295@gmail.com> Co-authored-by: Austin Hampton <amh107@latech.edu>
Closes #1213
Closes #1214
PR Description
Add capability for dataPut and dataGet to pass appropriate information required for mapped collections into tasks, and make Core utilize these new fields when processing Globus transfer tasks.
Additionally, extract some functionality to a more MVC-compliant framework for better reusability of introduced code.
Tasks
Summary by Sourcery
Adds support for mapped collection tokens, allowing dataPut and dataGet to pass collection-specific information to tasks, and updates Core to utilize these fields when processing Globus transfer tasks. It also refactors some functionality into a more MVC-compliant framework for better reusability.
New Features:
Tests: