Skip to content

Commit

Permalink
Allow only users to delete their own interviews because
Browse files Browse the repository at this point in the history
not all authors will give their ALKiln account admin permissions,
(which is what we recommend anyway) so they can only delete their
own interviews
  • Loading branch information
plocket committed Jun 21, 2024
1 parent eee17f0 commit 0308657
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 100 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ Format:

### Fixed

- Now delete interviews upon interview completion
- Now delete interviews upon interview completion for signed in users

### Internal

- Test deleting interviews upon interview completion
- Test deleting interviews upon interview completion for signed in users
- Added @temp_error to neutralize tests that are getting undesired errors right now, but which we anticipate being fixed soon. We want to ignore them for a short while. For example, an upstream change temporarily breaking some tests.

## [5.12.0] - 2024-06-06
Expand Down
15 changes: 5 additions & 10 deletions docassemble/ALKilnTests/data/sources/da_api.feature
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,18 @@
Feature: I use the docassemble API

@fast @da1 @signin @sessions @delete
Scenario: After signing in, I delete an interview
Given I sign in with the email "USER1_EMAIL" and the password "USER1_PASSWORD"
Scenario: After signing in, I successfully delete an interview
Given I sign in with the email "USER1_EMAIL", "USER1_PASSWORD", "USER1_API_KEY"
And I start the interview at "all_tests"
# Must continue to get interview into "My interviews" list
And I tap to continue
And I start the interview at "all_tests"
# Must continue to get interview into "My interviews" list
And I tap to continue
Then of two interviews, I delete one __internal__

@fast @da2 @sessions @delete
Scenario: Without signing in, I delete an interview
Scenario: As an anonymous user, I silently fail to delete an interview
Given I start the interview at "all_tests"
# Must continue to get interview into "My interviews" list
And I tap to continue
And I start the interview at "all_tests"
# Must continue to get interview into "My interviews" list
# Must continue to theoretically get interview into "My interviews" list
And I tap to continue
Then of two interviews, I delete one __internal__
#Then there are no interview_vars saved __internal__
Then as an anonymous user, I fail to delete an interview and get no error __internal__
16 changes: 8 additions & 8 deletions lib/docassemble/docassemble_api_REST.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,16 @@ da.delete_project = async function ( timeout ) {


// node -e 'require("./lib/docassemble/docassemble_api_REST.js").delete_interview(...)'
da.delete_interview = async function ( timeout, { uid = null }) {
da.delete_interview = async function ( timeout, { session = null, api_key = null }) {
/* Delete an interview with the given session id. */
let options = {
method: 'DELETE',
headers: { 'X-API-Key': session_vars.get_dev_api_key() },
headers: { 'X-API-Key': api_key },
// Can't use `api/session` that uses the API key to identify the user's
// interview sessions. The test may have signed in with an account
// that's different than the ALKiln dev account that sends this request.
url: `${ session_vars.get_da_server_url() }/api/interviews`,
params: { session: uid },
url: `${ session_vars.get_da_server_url() }/api/user/interviews`,
params: { session },
timeout: timeout,
};

Expand All @@ -76,16 +76,16 @@ da.delete_interview = async function ( timeout, { uid = null }) {
}; // Ends da.delete_interview()

// node -e 'require("./lib/docassemble/docassemble_api_REST.js").get_interview(...)'
da.get_interview = async function ( timeout, { uid = null }) {
da.get_interview = async function ( timeout, { session = null, api_key = null }) {
/* Get a specific interview. */
let options = {
method: 'GET',
headers: { 'X-API-Key': session_vars.get_dev_api_key() },
headers: { 'X-API-Key': api_key },
// Can't use `/api/user/interviews` that uses the API key to identify the user's
// interview sessions. The test may have signed in with an account
// that's different than the ALKiln dev account that sends this request.
url: `${ session_vars.get_da_server_url() }/api/interviews`,
params: { session: uid },
url: `${ session_vars.get_da_server_url() }/api/user/interviews`,
params: { session },
timeout: timeout,
};

Expand Down
121 changes: 81 additions & 40 deletions lib/docassemble/docassemble_api_interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,48 +201,75 @@ da_i.delete_project = async function ( delete_options ) {
}; // Ends da_i.delete_project()

// node -e 'require("./lib/docassemble/docassemble_api_interface.js").quietly_try_to_delete_interviews(...)'
da_i.quietly_try_to_delete_interviews = async function ({ interviews = {}, options = {} }) {
/** Delete a user's interview. Do we need to watch for a timeout? */
da_i.quietly_try_to_delete_interviews = async function ({ interviews = {}, api_keys = [], options = {} }) {
/** Try to delete the given interviews. Fail silently. Example failure: the test
* did not log in any users and the testing account's role prevents
* access to other user's interviews. Do we need to watch for a timeout? */

// Defaults
let { timeout } = options || { timeout: DEFAULT_MAX_REQUEST_MS };

let responses = [];
for ( let id in interviews ) {
let interview_vars = interviews[ id ];
if ( interview_vars && interview_vars.uid ) {
try {
log.debug({ type: `da api info`, code: `ALK0188`, pre: `Trying to delete the interview with the id ${ interview_vars.uid }.` });

// Actual work:
let response = await _da_REST.delete_interview( timeout, interview_vars );
log.debug({ type: `da api info`, code: `ALK0189`,
pre: `No error when trying to delete interview ${ interview_vars.uid } at ${ interview_vars.i }.`
});

} catch ( error ) {
error.interview_vars = interview_vars;
let msg = `Error when deleting the interview. See https://docassemble.org/docs/api.html#session_delete.`
log.debug({ type: `da api info`, code: `ALK0190`, pre: msg, data: error });
}
let interview = interviews[ id ];
if ( interview && interview.uid ) {
let sub_responses = await da_i.quietly_delete_one_interview( timeout, {
interview,
api_keys
});
responses.push( sub_responses );
} else {
log.debug({ type: `da api info`, code: `ALK0191`, pre: `There is no interview to delete.` });
}
}
return responses;

}; // Ends da_i.quietly_try_to_delete_interviews()

// node -e 'require("./lib/docassemble/docassemble_api_interface.js").quietly_delete_one_interview(...)'
da_i.quietly_delete_one_interview = async function ( timeout, { interview = {}, api_keys = [] }) {
/** Try to delete an interview using all available API keys. Fail silently.
* Example failure: the test did not log in any users and the testing
* account's role prevents access to other user's interviews. */
api_keys = normalize_api_keys( api_keys );
let responses = [];
for ( let api_key of api_keys ) {
try {
log.debug({ type: `da api info`, code: `ALK0188`,
pre: `Trying to delete the interview with the id ${ interview.uid } and an API key.`
});

// Actual work:
let response = await _da_REST.delete_interview( timeout, {
session: interview.uid,
api_key
});
responses.push( response );

log.debug({ type: `da api info`, code: `ALK0189`,
pre: `No error when trying to delete interview ${ interview.uid } at ${ interview.i }.`
});
} catch ( error ) {
error.interview = interview;
let msg = `Error when deleting the interview. See https://docassemble.org/docs/api.html#session_delete.`
log.debug({ type: `da api info`, code: `ALK0190`, pre: msg, data: error });
}
} // ends for api keys
return responses;
}; // Ends da_i.quietly_delete_one_interview()

// node -e 'require("./lib/docassemble/docassemble_api_interface.js").get_interviews(...)'
da_i.get_interviews = async function ({ interviews = {}, options = {} }) {
da_i.get_interviews = async function ({ interviews = {}, api_keys = [], options = {} }) {
/** Return an object of interviews that exist on the server
* that match the given interviews. Keys are the session ids. */
// Defaults
let { timeout } = options || { timeout: DEFAULT_MAX_REQUEST_MS };

api_keys = normalize_api_keys( api_keys );

let fetched_interviews = {};
if ( interviews ) {
for ( let id in interviews ) {
let interview_vars = interviews[ id ];
let fetched = await da_i.get_interview({ interview_vars });
let interview = interviews[ id ];
let fetched = await da_i.get_interview({ interview, api_keys, options });
if ( fetched && fetched.session ) {
fetched_interviews[ fetched.session ] = fetched;
}
Expand All @@ -253,34 +280,48 @@ da_i.get_interviews = async function ({ interviews = {}, options = {} }) {
}; // Ends da_i.get_interviews()

// node -e 'require("./lib/docassemble/docassemble_api_interface.js").get_interview(...)'
da_i.get_interview = async function ({ interview_vars, options = {} }) {
da_i.get_interview = async function ({ interview = {}, api_keys = [], options = {} }) {
/** Try to get an interview from the server that matches the given interview data. */

// Defaults
let { timeout } = options || { timeout: DEFAULT_MAX_REQUEST_MS };
api_keys = normalize_api_keys( api_keys );

if ( interview_vars && interview_vars.uid ) {
try {
log.debug({ type: `da api info`, code: `ALK0192`, pre: `Trying to get the interview with the id ${ interview_vars.uid }.` });
if ( interview && interview.uid ) {
for ( let api_key of api_keys ) {
try {
log.debug({ type: `da api info`, code: `ALK0192`, pre: `Trying to get the interview with the id ${ interview.uid } and an API key.` });

// Actual work:
let response = await _da_REST.get_interview( timeout, interview_vars );
if ( response && response.data && response.data.items && response.data.items[0] && response.data.items[0].session ) {
log.debug({ type: `da api info`, code: `ALK0193`, pre: `Status ${ response.status }. Got ${ response.data.items[0].session }.`});
return response.data.items[0];
} else {
log.debug({ type: `da api info`, code: `ALK0194`, pre: `Status ${ response.status }. Cannot find ${ interview_vars.uid }`});
}
// Actual work:
let response = await _da_REST.get_interview( timeout, {
session: interview.uid,
api_key
});

} catch ( error ) {
error.interview_vars = interview_vars;
let msg = `${ error.status } "${ error.statusText }" Ran into an error when getting the interview ${ interview_vars.uid }. See https://docassemble.org/docs/api.html#interviews.`
log.debug({ type: `da api info`, code: `ALK0195`, pre: msg, data: error });
}
if ( response && response.data && response.data.items && response.data.items[0] && response.data.items[0].session ) {
log.debug({ type: `da api info`, code: `ALK0193`, pre: `Status ${ response.status }. Got ${ response.data.items[0].session }.`});
return response.data.items[0];
} else {
log.debug({ type: `da api info`, code: `ALK0194`, pre: `Status ${ response.status }. Cannot find ${ interview.uid }`});
}
} catch ( error ) {
error.interview = interview;
let msg = `${ error.status || `No error.status.` } ${ error.statusText || error.data || `No error.statusText.` } Ran into an error when getting the interview ${ interview.uid }. See https://docassemble.org/docs/api.html#interviews.`
log.debug({ type: `da api info`, code: `ALK0195`, pre: msg, data: error });
}
} // ends for api_keys
} else {
log.debug({ type: `da api info`, code: `ALK0196`, pre: `There is no interview to get.` });
}

return null;

}; // Ends da_i.get_interview()

let normalize_api_keys = function ( api_keys = [] ) {
/** Ensure the dev API key is in the api_keys list. */
let dev_key = session_vars.get_dev_api_key();
if ( !api_keys ) { api_keys = [ dev_key ]; }
if ( !api_keys.includes( dev_key ) ) { api_keys.unshift( dev_key ); }
return api_keys;
};
14 changes: 13 additions & 1 deletion lib/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -2938,7 +2938,9 @@ module.exports = {
await scope.afterStep(scope, {waitForShowIf: true});
}, // Ends scope.steps.set_al_address()

sign_in: async function ( scope, { email_secret_name, password_secret_name }) {
sign_in: async function ( scope, {
email_secret_name, password_secret_name, api_key_secret_name
}) {
/** Allow the developer to log a user into their server using GitHub
* secrets to authenticate.
*/
Expand Down Expand Up @@ -2991,6 +2993,16 @@ module.exports = {
if ( password === undefined ) {
reports.addToReport( scope, { type: `error`, code: `ALK0162`, value: password_msg })
}
// Save API key to clean up interviews later. Nice to have, not critical
if ( api_key_secret_name ) {
let api_key = process.env[ api_key_secret_name ];
let api_key_msg = `The api_key GitHub SECRET "${ api_key_secret_name }" doesn't exist. Check for a typo and check your workflow file.`
if ( api_key === undefined ) {
reports.addToReport( scope, { type: `warning`, code: `ALK0199`, value: api_key_msg });
} else {
scope.scenarios.get( scope.scenario_id ).api_keys.push( api_key );
}
}

expect( email, email_msg ).to.not.equal( undefined );
expect( password, password_msg ).to.not.equal( undefined );
Expand Down
Loading

0 comments on commit 0308657

Please sign in to comment.