Skip to content

lore-server: Support file:// JWKS endpoints in JWK service #44

Open
kidskoding wants to merge 4 commits into
EpicGames:mainfrom
kidskoding:main
Open

lore-server: Support file:// JWKS endpoints in JWK service #44
kidskoding wants to merge 4 commits into
EpicGames:mainfrom
kidskoding:main

Conversation

@kidskoding

@kidskoding kidskoding commented Jun 20, 2026

Copy link
Copy Markdown

Summary

Fixes #32

Added a scheme check in fetch_new_keys() that checks for either file:// or http:// (https://)

  • If http:// or https://, nothing changes - same network fetch as before
  • If it's a file://, open the path on disk and read contents directly (implemented via tokio::fs::read_to_string instead of communicating over HTTP)

Test Plan

Added two tests covering the new file-read path: one verifies keys load correctly from a file:// endpoint and the other confirms a missing file still fails cleanly

  • Ran Tests: cargo test -p lore-server auth::jwk:: — both loads_keys_from_file_url and file_url_missing_file_returns_error - PASS
  • Format, Build, and Pre-commit - Formatted via clippy and cargo nightly formatter; Built via cargo build; pre-commit tool ran
    • cargo +nightly fmt --all
    • cargo clippy --all-targets -- -D warnings --no-deps
    • cargo build — all clean (no error)
    • pre-commit run --all-files — clean

@kidskoding

kidskoding commented Jun 21, 2026

Copy link
Copy Markdown
Author

maintainers / someone please review - @ragnarula @mjansson

mjansson
mjansson reviewed Jun 22, 2026
View reviewed changes
Comment thread lore-server/src/auth/jwk.rs Outdated
peter-lockhart-pub
peter-lockhart-pub reviewed Jun 22, 2026
View reviewed changes

@peter-lockhart-pub peter-lockhart-pub left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you very much for the contribution, this looks good to me. Can I be a bit pedantic please; could you move the .gitignore change to another PR? Just so the commit remains meticulous and free from unrelated changes?

@kidskoding

kidskoding commented Jun 22, 2026

Copy link
Copy Markdown
Author

sure thing! accidentally stashed all changes at once!

@kidskoding
kidskoding force-pushed the main branch 2 times, most recently from b2cdd30 to b6acd90 Compare June 22, 2026 08:58
@kidskoding kidskoding mentioned this pull request Jun 22, 2026
Closed
@kidskoding

kidskoding commented Jun 22, 2026

Copy link
Copy Markdown
Author

@peter-lockhart-pub please review again, along with #50!

peter-lockhart-pub
peter-lockhart-pub approved these changes Jun 22, 2026
View reviewed changes

@peter-lockhart-pub peter-lockhart-pub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@kidskoding

kidskoding commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Maintainers may need to reapprove CI/CD workflows due to a force push of commits on existing fork, intended to align with relevant PR titles - @mjansson, @peter-lockhart-pub

@peter-lockhart-pub

peter-lockhart-pub commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Maintainers may need to reapprove CI/CD workflows due to a force push of commits on existing fork, intended to align with relevant PR titles - @mjansson, @peter-lockhart-pub

Done! This PR will be picked up by another process that will merge it now that it is approved and good to go

@valentina2509 valentina2509 mentioned this pull request Jun 26, 2026
Open
Anirudh Konidala added 2 commits July 6, 2026 20:28
lore-server: support file:// JWKS endpoints in JWK service
f1e0507 
Signed-off-by: Anirudh Konidala <kidskodingclub@gmail.com>
lore-server: Split JWKS parse-error warning by endpoint scheme
00dfe9a 
Per review feedback, the file:// and http(s):// JWKS load paths shared one generic parse-error
  warning. Split it so the file case logs "invalid JWKS file contents" and the remote case keeps "failed to parse JWKS response".

Signed-off-by: Anirudh Konidala <kidskodingclub@gmail.com>
@mjansson

mjansson commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

We'll get this merged once the intake and attribution process is in place, ideally early next week.

@kidskoding

kidskoding commented Jul 14, 2026

Copy link
Copy Markdown
Author

sounds good! thank you! updated fork so looks like PR has updated merge commit in order to sync fork to upstream

epic-lore-bot Bot pushed a commit that referenced this pull request Jul 16, 2026
gitignore: Ignore entire .claude/ directory
370536a 
Upon request in #44 thread by @peter-lockhart-pub 

Local Claude Code skills and settings live under .claude/ and shouldn't be tracked. The prior rule only covered one skill subdirectory; broaden it to the whole directory

Just an fyi, these skills that were gitignored are used to help assist in the codebase, such as finding good issues to tackle and understanding what the codebase does from a technical standpoint. Some do assist in code, where disclaimers will be written when done so, but code will still carefully be checked, reviewed, and tested via a human before sending in a PR

```
Imported-PR: #50
Imported-From: 46da301
Imported-Base: a00b1c4
Imported-Merge: 9595422
Imported-Author: Anirudh Konidala
Signed-off-by: Anirudh Konidala <kidskodingclub@gmail.com>
GH-URL: #50
```

Lore-RevId: 316
Lore-Signature: 19ef4eb8b9d9a88b8fe719aa4ebae4e4ba91378645848dc2a255cdff96a89203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mjansson mjansson mjansson left review comments
@peter-lockhart-pub peter-lockhart-pub peter-lockhart-pub approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

JWK key ring URL does not support "file://"

3 participants

@kidskoding @peter-lockhart-pub @mjansson