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

OEP-42 and XBlock authentication #489

Open
robrap opened this issue May 30, 2023 · 9 comments
Open

OEP-42 and XBlock authentication #489

robrap opened this issue May 30, 2023 · 9 comments
Labels
help wanted Ready to be picked up by anyone in the community

Comments

@robrap
Copy link
Contributor

robrap commented May 30, 2023

To Do item, post discussion:

As per the comment #489 (comment),

This may be another case where we just note in the OEP that XBlock authentication is not under development (or however you want to put it), and point to this ticket for more details.

So whomever is picking up this task, please add this note appropriately, point to this ticket, and update the OEP changelog. Thanks!

Original ticket description

Minor context: I just learned (or re-learned) about XBlock secure tokens used for authentication, implemented in the XBlock utils here. Note: this is old enough that for all I know I took part in reviews and just don't remember.

Here are some assorted thoughts related to this, that may or may not result in different tickets:

  • At a minimum, we should update OEP-42 on Authentication to point to the best docs around XBlock authentication. This OEP is used in part as an index page to other authentication documentation about how AuthN works in edx-platform.
  • I did not see an ADR around this decision. It is possible that someone could retroactively create an ADR that we point to. Otherwise, we could point to docstrings like the one linked above in the implementation.
    • There seems to have been discussion somewhere around these tokens living for 2-4 days, and it would be great to capture some of that discussion and alternatives discussed, etc.
  • Related, I sort of thought we had an undocumented semi-made decision to no longer user secret keys in general, and instead rely on asymmetrically signed JWTs to improve security and potential key rotation. Maybe this solution isn't sharing the secret key, so it doesn't matter? If there were an ADR, it would be interesting to see this discussed. Also, does this warrant a separate ticket and possibly a separate OEP around shared secret keys?
@robrap
Copy link
Contributor Author

robrap commented May 30, 2023

FYI: @bradenmacdonald

@bradenmacdonald
Copy link
Contributor

@robrap Sorry for not writing an ADR about it. I did try to write an extensive docstring though as you can see. I think that if we decide to keep this, a retroactive ADR makes a lot of sense.

One important clarification is that that implementation and auth mechanism were only used by the Blockstore XBlock runtime which was only used for LabXchange, though may soon be used for Content Libraries V2 as well. So in some sense it is an experimental approach, to test stronger XBlock sandboxing. I think it has been shown to work well, but there is certainly room for a larger discussion around it before we decide if it's a useful auth mechanism to move forward with for sandboxed XBlocks, or if we should use something else.

There are no shared secret keys in this approach, and keys can be rotated whenever you want. The "secret" lives on the server and is used to generate a user-specific hashed token (asymmetrically hashed) which will work until it expires or the server's secret is rotated.

@kenclary has also been looking into this a lot lately and may have additional input about next steps.

@robrap
Copy link
Contributor Author

robrap commented May 30, 2023

Thanks @bradenmacdonald. No worries.

  1. The docstrings are a huge help.
  2. I’m not entirely clear on the mechanism planned for sandboxed XBlocks, but I assume the endpoint returning the secure token first relies on typical auth, and then this secure token is somehow passed to an iframe to be used for further calls?
  3. It sounded like this implementation was being taken as the de facto way of doing this, vs something to be considered and discussed with a new ADR. Maybe I got the wrong impression though?

@bradenmacdonald
Copy link
Contributor

I’m not entirely clear on the mechanism planned for sandboxed XBlocks, but I assume the endpoint returning the secure token first relies on typical auth, and then this secure token is somehow passed to an iframe to be used for further calls?

Yes, exactly. The JavaScript code running within the iframe has no way to make authenticated calls to the LMS other than via this token, and this token only works for calls to the XBlock runtime API that are scoped to the particular block in the iframe. So it cannot access any other LMS APIs nor user data, which is the key thing.

I certainly assumed there would be some discussion on the merits of this approach and relevant alternatives before it is more widely adopted or blessed.

@sarina
Copy link
Contributor

sarina commented May 17, 2024

Closing this for no action in a year. Please reopen with more detail about current work to be done if you are picking it up or are asking someone else to do so.

@sarina sarina closed this as completed May 17, 2024
@sarina sarina added the closed inactivity PR was closed because the author abandoned it label May 17, 2024
@robrap
Copy link
Contributor Author

robrap commented May 17, 2024

@bradenmacdonald: What is the current status of this? I don't think we need to reopen unless someone makes progress on this, but at least it is somewhat documented somewhere by having this issue.

@bradenmacdonald
Copy link
Contributor

@robrap As far as I know, there's no related activity happening now, though at some point in the next six months we're going to need to figure out a better solution for rendering XBlocks in MFEs, and then this may be relevant to the discussion.

@robrap
Copy link
Contributor Author

robrap commented May 21, 2024

@sarina:

  1. This may be another case where we just note in the OEP that XBlock authentication is not under development (or however you want to put it), and point to this ticket for more details.
  2. I'm not sure who needs to be aware of this ticket. Maybe @arbrandes? Others?

@sarina
Copy link
Contributor

sarina commented May 23, 2024

OK, re-opening as an issue to add that note.

@sarina sarina reopened this May 23, 2024
@sarina sarina added help wanted Ready to be picked up by anyone in the community and removed closed inactivity PR was closed because the author abandoned it labels May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Ready to be picked up by anyone in the community
Projects
None yet
Development

No branches or pull requests

3 participants