Skip to content
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

Boxcarring #131

Closed
wants to merge 10 commits into from
Closed

Boxcarring #131

wants to merge 10 commits into from

Conversation

alexolivier
Copy link
Contributor

  • Moves existing app and backend into a spec-1.0 folder
  • Extended application with boxcarring introduced in spec-1.1 folder
  • Added v1.0 and v1.1 sections into documentation site

Signed-off-by: Alex Olivier <[email protected]>
Signed-off-by: Alex Olivier <[email protected]>
Signed-off-by: Alex Olivier <[email protected]>
Copy link

netlify bot commented Aug 13, 2024

‼️ Deploy request for authzen-todo rejected.

Name Link
🔨 Latest commit 2560988

@davidjbrossard
Copy link
Collaborator

@alexolivier I merged main into your branch. I had made some minor text edits on the API spec.

Copy link
Collaborator

@ogazitt ogazitt left a comment

Choose a reason for hiding this comment

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

I've been thinking about how much commonality could we have between the v1.0 and v1.1 scenarios.

For authzen-interop-website, clearly a single project will work. The site organization I envisioned could add more scenarios (e.g., in addition to todo, it could have todo-boxcarred). And the results pages could be factored accordingly.

For authzen-todo-application, I agree that we need two different apps, unless we add some kind of selector in the app that tells us which behavior we want. So I think the work you did here makes sense.

For authzen-todo-backend, I'm torn. Seems like adding another route (DELETE /todos) could be done in an additive way to the existing todo backend project. I like the middleware you created - I think you could just add it to the existing project without breaking anything?

The only question is the list of pdps returned (i.e. pdps.json). I could see how we could add another endpoint (GET /v1.1/pdps) and have a separate pdps.json file for the 1.1-compliant PDPs.

@alexolivier
Copy link
Contributor Author

alexolivier commented Aug 20, 2024

For authzen-todo-application, I agree that we need two different apps, unless we add some kind of selector in the app that tells us which behavior we want. So I think the work you did here makes sense.

I can certainly do this - thinking some sort of feature toggle - but I will need to refactor it quite a bit which I'm happy to do if you are OK with it.

For authzen-todo-backend, I'm torn. Seems like adding another route (DELETE /todos) could be done in an additive way to the existing todo backend project. I like the middleware you created - I think you could just add it to the existing project without breaking anything?

That makes sense - I'll merge the backends

Signed-off-by: Alex Olivier <[email protected]>
Signed-off-by: Alex Olivier <[email protected]>
@alexolivier alexolivier marked this pull request as ready for review August 20, 2024 18:02
@alexolivier
Copy link
Contributor Author

backends merged but frontends are kept separate by version for now

@ogazitt
Copy link
Collaborator

ogazitt commented Sep 14, 2024

@alexolivier great work!

Given our recent spec changes, can you please update the PR to use an array for evaluations, and also the payloads for subject and resource should have subject.userID -> subject.properties.userID, and resource.ownerID -> resource.properties.ownerID.

@davidjbrossard
Copy link
Collaborator

davidjbrossard commented Sep 15, 2024 via email

@alexolivier
Copy link
Contributor Author

alexolivier commented Sep 17, 2024

Given our recent spec changes, can you please update the PR to use an array for evaluations, and also the payloads for subject and resource should have subject.userID -> subject.properties.userID, and resource.ownerID -> resource.properties.ownerID.

Sure I'll pick this up next week when I'm back from PTO

@ogazitt
Copy link
Collaborator

ogazitt commented Sep 26, 2024

superseded by PR #156

@ogazitt ogazitt closed this Sep 26, 2024
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.

3 participants