-
-
Notifications
You must be signed in to change notification settings - Fork 371
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
Use DocApiKey as valid ApiKeys #1115
Changes from all commits
637d3c9
b75e567
554cf1d
2ff7e67
cfc2c96
14f97f5
850363c
e4880d4
e97f75b
94386cd
b597a3d
5a631cb
0f425f4
8c0c822
c4eaa4a
8849d0e
50e58db
64abd51
798234f
d7d4c74
9e779f9
b5e2fec
a713ec4
1c6c734
3cda9e8
41b89ff
e5e049e
766a328
4cf4b2c
3ab5f95
ea05508
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -21,6 +21,7 @@ import {addPermit, clearSessionCacheIfNeeded, getDocScope, getScope, integerPara | |||||||||||||||||||||||||||||||
isParameterOn, optStringParam, sendOkReply, sendReply, stringParam} from 'app/server/lib/requestUtils'; | ||||||||||||||||||||||||||||||||
import {IWidgetRepository} from 'app/server/lib/WidgetRepository'; | ||||||||||||||||||||||||||||||||
import {getCookieDomain} from 'app/server/lib/gristSessions'; | ||||||||||||||||||||||||||||||||
import { ShareOptions } from 'app/common/ShareOptions'; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// exposed for testing purposes | ||||||||||||||||||||||||||||||||
export const Deps = { | ||||||||||||||||||||||||||||||||
|
@@ -324,6 +325,118 @@ export class ApiServer { | |||||||||||||||||||||||||||||||
return sendOkReply(req, res); | ||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// POST /api/docs/:did/apiKey | ||||||||||||||||||||||||||||||||
this._app.post('/api/docs/:did/apiKey', expressWrap(async (req, res) => { | ||||||||||||||||||||||||||||||||
const did = req.params.did; | ||||||||||||||||||||||||||||||||
const doc = await this._dbManager.getDoc(req); | ||||||||||||||||||||||||||||||||
if (!doc){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`No such doc: ${did}`, 404); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const linkId = req.body.linkId; | ||||||||||||||||||||||||||||||||
const query = await this._dbManager.getDocApiKeyByLinkId(did, linkId); | ||||||||||||||||||||||||||||||||
if (query){ | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpicking:
Suggested change
Same for below. In vim: For
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prettier or riot. :p There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eslint has some rules I think for that stuff, but still, I agree with a tool that would prevent us from making style-related comments. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe configuring eslint this way:
|
||||||||||||||||||||||||||||||||
throw new ApiError("LinkId must be unique", 400); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
const options = sanitizeDocApiKeyOptions(req.body.options); | ||||||||||||||||||||||||||||||||
req.body.options = JSON.stringify(options); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const key = await this._dbManager.createDocApiKey(did, req.body); | ||||||||||||||||||||||||||||||||
Comment on lines
+341
to
+344
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer avoiding altering the parameters,
Suggested change
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
return sendOkReply(req, res, key); | ||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// GET /api/docs/:did/apiKey/:linkId | ||||||||||||||||||||||||||||||||
this._app.get('/api/docs/:did/apiKey/:linkId', expressWrap(async (req, res) => { | ||||||||||||||||||||||||||||||||
const did = req.params.did; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const doc = await this._dbManager.getDoc(req); | ||||||||||||||||||||||||||||||||
if (!doc){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`No such doc: ${did}`, 404); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
const linkId = req.params.linkId; | ||||||||||||||||||||||||||||||||
const query = await this._dbManager.getDocApiKeyByLinkId(did, linkId); | ||||||||||||||||||||||||||||||||
return query ? sendOkReply(req, res, query) : res.status(404).send(); | ||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// PATCH /api/docs/:did/apiKey/:linkId | ||||||||||||||||||||||||||||||||
this._app.patch('/api/docs/:did/apiKey/:linkId', expressWrap(async (req, res) => { | ||||||||||||||||||||||||||||||||
const did = req.params.did; | ||||||||||||||||||||||||||||||||
const linkId = req.params.linkId; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const doc = await this._dbManager.getDoc(req); | ||||||||||||||||||||||||||||||||
if (!doc){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`No such doc: ${did}`, 404); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if (req.body.docId){ | ||||||||||||||||||||||||||||||||
throw new ApiError("Can't update DocId", 400); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may control and throw what the user passes using the convenient I can explain you how to do that after lunch. |
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if (req.body.key){ | ||||||||||||||||||||||||||||||||
throw new ApiError("Can't update key", 400); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const queryLinkId = await this._dbManager.getDocApiKeyByLinkId(did, req.body.linkId); | ||||||||||||||||||||||||||||||||
if (queryLinkId){ | ||||||||||||||||||||||||||||||||
throw new ApiError("LinkId must be unique", 400); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// In order to catch {options: ""} case | ||||||||||||||||||||||||||||||||
if (Object.keys(req.body).includes("options")){ | ||||||||||||||||||||||||||||||||
const options = sanitizeDocApiKeyOptions(req.body.options); | ||||||||||||||||||||||||||||||||
req.body.options = JSON.stringify(options); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const query = await this._dbManager.updateDocApiKeyByLinkId(did, linkId, req.body); | ||||||||||||||||||||||||||||||||
Comment on lines
+385
to
+391
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I wonder if we could not adapt or use inheritance for
|
||||||||||||||||||||||||||||||||
return sendOkReply(req, res, query); | ||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// DELETE /api/docs/:did/apiKey/:linkId | ||||||||||||||||||||||||||||||||
this._app.delete('/api/docs/:did/apiKey/:linkId', expressWrap(async (req, res) => { | ||||||||||||||||||||||||||||||||
const did = req.params.did; | ||||||||||||||||||||||||||||||||
const linkId = req.params.linkId; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const doc = await this._dbManager.getDoc(req); | ||||||||||||||||||||||||||||||||
if (!doc){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`No such doc: ${did}`, 404); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const linkId4Did = await this._dbManager.getDocApiKeyByLinkId(did, linkId); | ||||||||||||||||||||||||||||||||
if (!linkId4Did){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`Invalid LinkId: ${linkId}`, 404); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const query = await this._dbManager.deleteDocApiKeyByLinkId(did, linkId); | ||||||||||||||||||||||||||||||||
return sendOkReply(req, res, query); | ||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// GET /api/docs/:did/apiKeys | ||||||||||||||||||||||||||||||||
this._app.get('/api/docs/:did/apiKeys', expressWrap(async (req, res) => { | ||||||||||||||||||||||||||||||||
const did = req.params.did; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const doc = await this._dbManager.getDoc(req); | ||||||||||||||||||||||||||||||||
if (!doc){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`No such doc: ${did}`, 404); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const query = await this._dbManager.getDocApiKeys(did); | ||||||||||||||||||||||||||||||||
return sendOkReply(req, res, query); | ||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// DELETE /api/docs/:did/apiKeys | ||||||||||||||||||||||||||||||||
this._app.delete('/api/docs/:did/apiKeys', expressWrap(async (req, res) => { | ||||||||||||||||||||||||||||||||
const did = req.params.did; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const doc = await this._dbManager.getDoc(req); | ||||||||||||||||||||||||||||||||
if (!doc){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`No such doc: ${did}`, 404); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are many copy-paste for this part. For that case (here duplication does not help much understanding what is done), what do you think of factorizing? async function assertDocExistsAndIsAccessible(did: string) {
const doc = await this._dbManager.getDoc(req);
if (!doc){
throw new ApiError(`No such doc: ${did}`, 404);
}
} Usage: await assertDocExistsAndIsAccessible(req); |
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const query = await this._dbManager.deleteDocApiKeys(did); | ||||||||||||||||||||||||||||||||
return sendOkReply(req, res, query); | ||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// PATCH /api/orgs/:oid/access | ||||||||||||||||||||||||||||||||
// Update the specified org acl rules. | ||||||||||||||||||||||||||||||||
this._app.patch('/api/orgs/:oid/access', expressWrap(async (req, res) => { | ||||||||||||||||||||||||||||||||
|
@@ -699,3 +812,28 @@ async function updateApiKeyWithRetry(manager: EntityManager, user: User): Promis | |||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
throw new Error('Could not generate a valid api key.'); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
function sanitizeDocApiKeyOptions(rawOptions: string): ShareOptions { | ||||||||||||||||||||||||||||||||
const legalOptions: (keyof ShareOptions)[] = ["access"]; | ||||||||||||||||||||||||||||||||
const legalAccessValues: ShareOptions["access"][] = ["editors", "viewers"]; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if (!rawOptions){ | ||||||||||||||||||||||||||||||||
throw new ApiError("Missing body params: options", 400); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const options = JSON.parse(rawOptions); | ||||||||||||||||||||||||||||||||
if (!options.access){ | ||||||||||||||||||||||||||||||||
throw new ApiError("Missing options: access", 400); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
Object.keys(options).forEach(element => { | ||||||||||||||||||||||||||||||||
if (!(legalOptions as string[]).includes(element)){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`Invalid option: ${element}`, 400); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if (!legalAccessValues.includes(options.access)){ | ||||||||||||||||||||||||||||||||
throw new ApiError(`Invalid access value: ${options.access}`, 400); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
return {...options, "apikey": true}; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may use |
||||||||||||||||||||||||||||||||
} |
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.
Nitpicking: