ueberauth / guardian

Elixir Authentication
MIT License
3.4k stars 381 forks source link

I would like to be able to set permissions from a DB table rather than hard coded in a permissions set. #709

Closed kairos0ne closed 1 day ago

kairos0ne commented 1 year ago

Problem Statement

I tried to get the permissions structured correctly for Guardian in my Guardian module like so.

defmodule Dynamic.Guardian do

  alias Dynamic.BaseContent
  alias Dynamic.BaseStructures
  alias Dynamic.Repo

  use Guardian, otp_app: :dynamic,
    permissions: BaseStructures.get_table_permissions!()

However I get this error:

(RuntimeError) could not lookup Ecto repo Dynamic.Repo because it was not started or it does not exist

Stacktrace:
  │ (ecto 3.8.4) lib/ecto/repo/registry.ex:22: Ecto.Repo.Registry.lookup/1
  │ (ecto 3.8.4) lib/ecto/repo/supervisor.ex:159: Ecto.Repo.Supervisor.tuplet/2
  │ (dynamic 0.1.0) lib/dynamic/repo.ex:2: Dynamic.Repo.all/2
  │ (dynamic 0.1.0) lib/dynamic/base_structures.ex:145: Dynamic.BaseStructures.get_table_permissions!/0
  │ lib/dynamic/guardian.ex:9: (module)Elixir

Solution Brainstorm

Any thoughts on how to support this or if Im in fact missing something fundamental.

yordis commented 1 year ago

You should not do a function call at compile time to find the permissions (Notice that you are calling a function).

That is why when you try to compile the code, it fails because you haven't started the repository and whatnot needed to call the database.

Your intent could be achieved by passing an MFA configuration that could be called to fetch the permissions instead, something like:

  use Guardian, otp_app: :dynamic,
    permissions: {BaseStructures, :get_table_permissions!, []}

I am not sure if this is supported yet, but a PR is welcomed.

geofflane commented 1 year ago
  use Guardian, otp_app: :dynamic,
    permissions: [BaseStructures, :get_table_permissions!, [])

I believe it's a tuple (threeple) {BaseStructures, :get_table_permissions!, []} see Guardian.Config.

But that won't work still because permissions are cached at compile time. (I just made it so only permissions are resolved at compile time instead of all config). That would need to be changed for this kind of resolution to work for permissions.

From what I'm seeing, the compile time pieces look like they might be optimizations that could just be read at runtime?

yordis commented 1 year ago

I believe it's a tuple (threeple) {BaseStructures, :get_table_permissions!, []} see Guardian.Config.

I fixed it; I meant to use a tuple instead of a list.

But that won't work still because permissions are cached at compile time.

I hear you. The idea is moving it at runtime 🤷🏻 otherwise, no way we could figure this one out as far as I can tell.

kairos0ne commented 1 year ago

I forked this repo as a POC to see if I could approach it with this capability, fork instead of PR at first because I wasn't sure if this was done intentionally or not. Perhaps for added security ??? not sure.

POC here: https://github.com/kairos0ne/guardian

Its a bit rudimentary - Added a update_permissions(perms) function. It can certainly can be improved.

      @spec update_permissions(permissions :: map) :: :ok
      def update_permissions(permissions) do
        Application.put_env(unquote(otp_app), __MODULE__, Keyword.put(config(), :permissions, permissions))
      end

Then you can call update_permissions from you Guardian module implementation.

I also changed the permissions module to support run time updates.

If you guys have any suggestions or improvements let me know.

yordis commented 1 year ago

I forked this repo as a POC to see if I could approach it with this capability, fork instead of PR at first because I wasn't sure if this was done intentionally or not. Perhaps for added security ??? not sure.

Please create a PR in Draft. That will help me to follow exactly what you have done so far since otherwise it is hard for me to follow the work.

kairos0ne commented 1 year ago

Sure I'll create a draft PR and a pull in all the changes np

yordis commented 10 months ago

@kairos0ne, any updates from your end?

kairos0ne commented 9 months ago

Let me take a look...

github-actions[bot] commented 2 weeks ago

This issue has been automatically marked as "stale:discard". If this issue still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment.

github-actions[bot] commented 1 day ago

Closing this issue after a prolonged period of inactivity. If this issue is still relevant, feel free to re-open the issue. Thank you!