ash-project / ash_paper_trail

The extension for keeping an audit log of changes to your Ash resources.
https://hexdocs.pm/ash_paper_trail
MIT License
32 stars 15 forks source link

allow all`Ash.Domain` authorization types #80

Closed tunchamroeun closed 3 months ago

tunchamroeun commented 3 months ago

https://hexdocs.pm/ash/dsl-ash-domain.html#authorization-authorize

:always | :by_default | :when_requested

Contributor checklist

zachdaniel commented 3 months ago

🤔 this would be a breaking change for folks relying on this behavior unfortunately.

I'm not entirely sure that it ever makes sense to authorize the creation of a version object, it's not something that is ever meant to fail. Authorization should be done on the action in question.

zachdaniel commented 3 months ago

Given that this would be a non-trivial breaking change, I think we should have this discussion in a new issue, and we can discuss how we can accomplish what you're looking to accomplish and what changes would be necessary. Thank you for your proposal 🙇

tunchamroeun commented 3 months ago

🤔 this would be a breaking change for folks relying on this behavior unfortunately.

I'm not entirely sure that it ever makes sense to authorize the creation of a version object, it's not something that is ever meant to fail. Authorization should be done on the action in question.

If I add

authorization do
  authorize :always
end

with authorize?: false on versions resource will not work on create action (you can test or do I miss something?)

tunchamroeun commented 3 months ago

Given that this would be a non-trivial breaking change, I think we should have this discussion in a new issue, and we can discuss how we can accomplish what you're looking to accomplish and what changes would be necessary. Thank you for your proposal 🙇

The code seems to enforce the required domain: Example.Domain behavior in ash3. However, I'd like to verify this further.

zachdaniel commented 3 months ago

Hmm...I don't think it would, as it pulls the domain off of the changeset in a hook: domain: changeset.domain,

tunchamroeun commented 3 months ago

I will reproduce the new issue about this.