enthought / pywin32-ctypes

A (partial) reimplementation of pywin32 in pure python
Other
18 stars 9 forks source link

question re CREDENTIAL __DO_NOT_USE_ fields in _authentication.py #105

Open jrobbins-LiveData opened 2 years ago

jrobbins-LiveData commented 2 years ago

I was wondering why _fields_ AttributeCount and Attribute are prefixed with __DO_NOT_USE_?

I have a potential use for setting an attribute on a Credentials Manager entry (to record the encoding used for a password), and was wondering if this __DO_NOT_USE_ had an back story about what kinds of issues I might be facing in using this feature?

https://github.com/enthought/pywin32-ctypes/blob/2db9303379e3d1714da556e3b58df6864cdd71f9/win32ctypes/core/ctypes/_authentication.py#L26

https://github.com/enthought/pywin32-ctypes/blob/28364ce0ac0d52dda3e4b46d568cd7fab3697074/win32ctypes/core/ctypes/_authentication.py#L36

https://github.com/enthought/pywin32-ctypes/blob/2db9303379e3d1714da556e3b58df6864cdd71f9/win32ctypes/core/ctypes/_authentication.py#L37

jdmarch commented 2 years ago

Hi @jrobbins-LiveData -- just a heads-up that it may be a week or two before @itziakos will be available to respond to your question.

jrobbins-LiveData commented 2 years ago

Thanks, I appreciate knowing that!

On Sun, Nov 28, 2021, 5:40 PM Jonathan March @.***> wrote:

Hi @jrobbins-LiveData https://github.com/jrobbins-LiveData -- just a heads-up that it may be a week or two before @itziakos https://github.com/itziakos will be available to respond to your question.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/enthought/pywin32-ctypes/issues/105#issuecomment-981165826, or unsubscribe https://github.com/notifications/unsubscribe-auth/AS43FGG5B7X5X3KCRIRL6V3UOKVVNANCNFSM5I5YSHEQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

itziakos commented 2 years ago

@jrobbins-LiveData It looks that the DO_NOT_USE prefix is a very early commit that was added when the ctypes implementation was first introduced. I suspect that it was probably because these keys where not needed at that time. Pywin32 does not have this limitation so we should fix that.

I am designating this report as a bug report. I will need to look into what is necessary to better support the application specific attributes. As far as I can tell it is not enough to just rename the attributes since Pywin32 has a different behaviour (you can probably use Pywin32 for development until we get this fixed).

jrobbins-LiveData commented 2 years ago

Hi @itziakos. In working on a keyring Windows issue, we had some discussions about augmenting the interface to the Cred<xyz> functions. Here are some of the factors involved:

I wrote a new implementation using ctypes. While it does exactly what I need to address the keyring issue, I can't speak for its general usefulness.

That said, if it, or something like it, would best belong in this project, I'd be happy to work on that with you. I kept the interface upwards compatible (I think?), so hopefully it wouldn't break anyone already using it. That would need to be verifed, of course!

Anyway, please let me know what you think.

itziakos commented 2 years ago

Thanks @jrobbins-LiveData

I had a quick look at the implementation in keyring. I will need to think how I can accomodate your usecase while keeping compatibility with the behaviour of pywin32. At the very least it should be easier to implement keyring's specific credential configuration on top of pywin32-ctypes (althought using a private module like _authenticate.py is still a small code smell).

On another note I noticed that in https://github.com/jaraco/keyring/blob/1f8bfb394d67e1f9a632916c444b60cbcebbb896/keyring/backends/Windows/api.py#L105 the code mutates the global windll.advapi32 object. I suggest that a local WinDDL instance is used to avoid mutating global state (which unfortunately a lot of libraries do).