SolidOS / SolidOS/solid-logic

Make authentication more flexible

Open
#291 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
TypeScript
Stars
12
Forks
13
Avg merge
15h 15m
Merged PRs (30d)
3

Description

When we switched out the auth lib there was an improvement suggestion from Noel:

Following up from my comments in this week's meeting, looking at the current PR it seems like src/authn/SolidAuthnLogic.ts is attempting to handle both Inrupt and Uvdsl libraries? If so, that's is good, here's a couple of comments:

I see that the Inrupt library was removed from the dependencies, but it's still used in some places using the "sessionAny". I'd strongly suggest to keep the dependency installed, and cast the variable to its actual type, rather than using any. Any should be avoided as much as possible.
The current implementation seems to rely on if/else logic inside of the same file. That is ok for now, specially if we can keep the external API for consumers the same. But ideally, we would have two different implementations of an interface, one for Inrupt and one for Uvdsl, and resolve the proper instance to use at runtime. This would also allow to add more adapters in the future. Even if that's not a goal right, it's also good for code organization. As Aad mentioned in the call, this is called the Adapter pattern. Though if that confuses you more than it helps, just take a look at how I've done it for my apps (each authentication library has its own class).
Another additional improvement is to make sure to lazy-load the authentication libraries. Basically, if someone is logging in with uvdsl, they don't need to download all the javascript of Inrupt's library. I also do that in my apps, in particular using dynamic imports (see the .lazy files).
I haven't looked at all the code in the PR, but it's not immediately clear to me how consumers of this library choose which authentication library to use. Ideally, we could have a login() function that takes an "authenticationLibrary" parameter (or "authenticator", as I call them in my code, etc.). That could default to uvdsl, but should be easy to replace with Inrupt.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading src/authn/SolidAuthnLogic.ts and the changes discussed in pull request 171 to understand the current Inrupt and Uvdsl handling. Define the desired consumer-facing selection and default behavior before assessing separate authenticator implementations and lazy loading. Done should preserve the external API while allowing the authentication library to be selected, defaulting to Uvdsl.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
authentication, backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.