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

[Planning] Major package refactor #33

Closed
ahmednfwela opened this issue Apr 11, 2023 · 3 comments
Closed

[Planning] Major package refactor #33

ahmednfwela opened this issue Apr 11, 2023 · 3 comments

Comments

@ahmednfwela
Copy link
Collaborator

ahmednfwela commented Apr 11, 2023

The package currently needs a major refactor in the following areas:

  1. launching a url and getting a response
    • this is currently inconsistent between web and io
    • I suggest adding 4 extra federated packages openidconnect_android, openidconnect_ios, openidconnect_macos, openidconnect_linux
    • each package might not necessarily need platform-dependent code, and some of them can use existing packages (e.g. url_launcher, flutter_appauth, etc...)
  2. OIDC discovery
    • while the package conforms to the spec for the most part, it needs extra unit tests
  3. Token persistence
    • The package is tightly coupled with flutter_secure_storage which isn't fully supported on web, so we need to decouple it and let each platform decide
  4. OpenIdIdentity
    • should be transformed to a plain object, and move persistence logic elsewhere
  5. Reorganizing the package to use exports instead of part/part of
  6. CI/CD on the github repo to automate package publishing
  7. Support Multi-tenancy
@jhancock4d
Copy link
Contributor

  1. There is a package for windows but it was removed because it just uses device code as does macos and linux right now which works basically the same to how they'd work anyhow. The reason I added a windows one was speculative because WinRT (WinUI now) has full captured browser window functionality for going to an IdP. My intent was to wait for FFI and then implement it in dart as I have in the past on Xamarin.

Not sure there is much of a win ever for linux and not just using device_code for linux. MacOS may have similar to iOS, I don't know. In that case it might be interesting to have device specific federation for it like windows.

Android and iOS were kept in the main package because there were pretty major issues with splitting them out when I wrote this. They could be split out now, though not sure if it really matters.

  1. flutter_secure_storage has as close to web support as any library (I wrote the functionality for flutter_secure_storage and contributed it specifically to make it work with this project because there was no secure storage for web anywhere else at the time. To keep as much code shared I'd strongly suggest that it stays there and any issues with the web integration in flutter_secure_storage be fixed (if possible... web doesn't do encrypted secured storage well at all)

  2. See above. The intent is to share all of that instead of having to maintain it separately. Don't reinvent the wheel if you don't need to. (and web crypto especially for this is really hard so we shouldn't be trying to maintain it in this library anyhow)

  3. Not sure what this buys anything. As soon as you use exports you're exposing stuff that should never be imported.

  4. 👍

@ahmednfwela
Copy link
Collaborator Author

@jhancock4d also since the package doesn't have a verified publisher on pub.dev
image
should we create a new domain for it? or just transform it to our publisher https://pub.dev/publishers/bdaya-dev.com/packages ?

@jhancock4d
Copy link
Contributor

Google disconnected the publisher on name change for some reason. It has been fixed.

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

No branches or pull requests

2 participants