Code Monkey home page Code Monkey logo

Comments (5)

dpopowich avatar dpopowich commented on June 30, 2024 1

If you don't want compatibility with the defined API...

All I'm saying is forcing identity to be a str by the assertion is arbitrary and limits the potential of the API. I believe the API will be more useful for wider usage than is currently allowed if identity is left an opaque type passed through to implementations.

from aiohttp-security.

Dreamsorcerer avatar Dreamsorcerer commented on June 30, 2024

I could be mistaken, as the details are not fresh in my mind, but that sounds like it makes it impossible to mix and match identity policies.

If you are passing some custom User object to remember() and a CookiesIdentityPolicy is used, what is it supposed to save into the cookie?

Expected behaviour, would be that you pass the user's ID (it is called an identity for a reason), which can then be remembered safely by any IdentityPolicy and your code should be able to load it from that on subsequent requests. e.g. If User is a sqlalchemy model, then you might do something like:
remember(request, resp, user.id)

Then in the other functions where you receive the identity, you would do something like user = session.get(User, identity).

from aiohttp-security.

dpopowich avatar dpopowich commented on June 30, 2024

While a trivial point, I note even your suggestion of remember(request, resp, user.id) doesn't work because User.id is not a string. I also note the implementation of the module-level remember() accepts arbitrary keyword args for, presumably, IdentityPolicy implementation needs, so there is some expectation of more than just a string being required to generate a token/header/cookie/etc.

As for mix&matching...why would I want to mix and match a home-grown, specific-to-my-application identity? Especially considering JWT which allows you to store arbitrary claims (i.e., by design JWT is meant to store far more than just an id).

Can remember() be wrapped to do what I want? Yes and that's what I'll have to do to use this API, but I note, except for the assertion that it's a string where I pointed out, the rest of the API makes no assumption as to the type or value of the identity and passes it around as an opaque object. I think the API will be more useful by not forcing an assumption of usage. And by making my identity an object, implementing the AuthorizationPolicy is soooo simple.

I note there's a protoype of a JWT IdentityPolicy in the source (undocumented -- considered not ready for primetime?) which doesn't implement remember() or forget(). It only implements identify(). Is the assumption that by some means outside the implementation the client has received a JWT, but this implementation knows how to decode? Strikes me that it makes more sense to tie encode/decoding together in one IdentityPolicy.

My implementation ties together an IdentityPolicy, AuthorizationPolicy, and middleware (processing requests with the Authorization header set, decoding and reconstituting the user instance), all working in harmonious beauty. ;-)

Meanwhile, here's what I'll have to do to wrap the stock remember if I want to continue with my usage:

import aiohttp_security as aiosec

async def remember(request, response, *, user, **claims):
   await aiosec.remember(request, response, '__unused__', user=user, **claims)

And then code my impl accordingly:

   async def remember(self, req, resp, identity, *, user, **claims):
      ...

from aiohttp-security.

Dreamsorcerer avatar Dreamsorcerer commented on June 30, 2024

While a trivial point, I note even your suggestion of remember(request, resp, user.id) doesn't work because User.id is not a string.

I often use the username as the ID (which is what is shown in most of the examples). If it's an int, you can still convert it easily with str() and int().

As for mix&matching...why would I want to mix and match a home-grown, specific-to-my-application identity? Especially considering JWT which allows you to store arbitrary claims (i.e., by design JWT is meant to store far more than just an id).

If you don't want compatibility with the defined API in order to have mix-and-match things, then I'm not sure what this library is actually offering you. You might as well just call identity_policy.remember() directly, there's not much else the library is going to do for you. That's pretty much all these functions do anyway: https://github.com/aio-libs/aiohttp-security/blob/master/aiohttp_security/api.py

It'll also make static typing a lot easier for you. I'm looking at adding annotations soon, and if we went with your approach, then all the functions would need to be annotated with Any and you would get no typing information, as there is no obvious way we can infer the type from the identity policy in these functions.

I note there's a protoype of a JWT IdentityPolicy in the source (undocumented -- considered not ready for primetime?) which doesn't implement remember() or forget(). It only implements identify(). Is the assumption that by some means outside the implementation the client has received a JWT, but this implementation knows how to decode? Strikes me that it makes more sense to tie encode/decoding together in one IdentityPolicy.

Seems to be deliberate: #147 (comment)

I think there is some mistunderstanding/disagreement about how the library is meant to work. I don't have enough in-depth knowledge right now to provide more useful feedback though.

from aiohttp-security.

toolforger avatar toolforger commented on June 30, 2024

The API accepts an identity, you want to send it a User which is identity plus password plus whatever may be appropriate for your use case.

The problem is that the library cannot know what the operations on your User object are. There could be some variation to equality, for example, and your calling code should know have to know whether the current or future versions of aiohttp-security may ever want to use equality (for example somebody might want to add some caching operations and uses the identity as a key, then you try caching and everything works, but it will fail in production because the wrong fields are compared).

It's the wrong mental model for "identity".
Identity is a dict (or database) key: it identifies all the data associated with a user, it does not contain them.
I.e. store the User objects outside aiohttp-security. If you need to deal with multiple identities and associated User objects, use an identity->User dict.

from aiohttp-security.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.