-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: validate store/add access #15
Conversation
View stack outputs
|
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.
Yes! I Like it
5603f06
to
51f5047
Compare
@alanshaw plz to add PR description or link to issue or both! |
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.
👍
carIsLinkedToAccount, | ||
carExists | ||
] = await Promise.all([ | ||
context.storeTable.exists(account, link.toString()), | ||
context.access.verifyInvocation(invocation), |
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.
do context.access.verifyInvocation
before we do any other work? db and bucket reads have a smol $ cost.
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.
IDK, I'm optimistically hoping that requests are going to have access to do this.
@@ -35,7 +39,9 @@ async function ucanInvocationRouter (request) { | |||
} | |||
} | |||
|
|||
const server = await createUcantoServer({ | |||
const id = await getServiceDid() |
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.
plz long name like
const id = await getServiceDid() | |
const uploadApiDid = await getServiceDid() |
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 extend this to store/*
? and maybe upload/*
if this lands after storacha/w3up#184
…age/upload-api into feat/validate-store-add-access
Stack outputs updated
|
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.
LGTM! 🎉 Thanks
@@ -77,3 +96,81 @@ export function getSigningOptions(ctx) { | |||
bucket: ctx.bucketName, | |||
} | |||
} | |||
|
|||
/** @returns {Promise<MockAccess>} */ | |||
export async function createAccessServer () { |
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.
Might be nice to consider creating a mock module for this as I expect it to be needed in more services. Not for now, just raising thoughts for visibility :)
I had to make similar changes to createUcantoServer in #35 so let's get this one merged and I can deal with the conflicts over there. |
This PR adds a request to w3access when a
store/add
invocation is handled.The request to w3access ensures that the target space has been verified by a user via email.