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

Discussion: Testing with PharIO\GnuPG #26

Open
sebastianfeldmann opened this issue Oct 13, 2023 · 9 comments
Open

Discussion: Testing with PharIO\GnuPG #26

sebastianfeldmann opened this issue Oct 13, 2023 · 9 comments
Labels
question Further information is requested

Comments

@sebastianfeldmann
Copy link
Contributor

I'm a bit stuck. I'm trying to fix the current tests but I keep running into problems.

PHPUnit\Framework\MockObject\IncompatibleReturnValueException: Method import may not return value of type array, its declared return type is "void"

Was playing around with it a bit but always running into expected GnuPg got PharIO\GnuPg issues when testing without the extension.

I was thinking about hiding the two GnuPG implementations behind a GnuPGProxy and only type hint and Mock the proxy. So I don't have to deal with the type jougling.

But I'm not sure if if makes sense to implement it in this repo.

Maybe implementing it in phar-io/gnupg makes more sense. Then even phive could implement against the proxy api und would not care about the \GnuPg or PharIO\GnuPg magic.
It would always be GnuPgProxy (name still up for discussion).

Or am I just too blind to figure this little type mess out another way.

@theseer @heiglandreas what do you think.

@sebastianfeldmann sebastianfeldmann added the question Further information is requested label Oct 13, 2023
@theseer
Copy link
Member

theseer commented Oct 14, 2023

We really just need to rework (to not say rewrite ;-) the gnupg library. I have no idea why I/we considered a good idea to implement it the way we did...

@heiglandreas
Copy link
Collaborator

IMO the GnuPH lib should donthe heavy lifting of figuring out whether the lib is available or not. So in essence that already should be the proxy.

How that is then handled internally is not an issue of this package 🙈

@heiglandreas
Copy link
Collaborator

We are already using a factory to fetch the GnuPG class.

I'd propose to use a GnuPG interface and the factory then returns one of three implementations: one based on the GnuPG extension, one based on the gpg exectutable and one fallback that will never validate (or it will but output an appropriate message?)

@sebastianfeldmann
Copy link
Contributor Author

The question would be if we hand out the implementations to users of the lib, or if we hide that stuff in the background.

I have to admit I'm still far from being an expert with all the gpg stuff. Maybe we define multiple interfaces depending on the use case.

PharIO\GnuPG\Verifyer
PharIO\GnuPG\KeyImporter
...

I think I'm in favor of hiding the "complexity" for lib users and internally take care of everything that is needed to make the key handling and verification work. So internally do exactly what you described only difference not handing over different implementations to the user instead handling them internally.

I would love to work on this (need to finally become at least decent with this gpg stuff). Happy to video call in the upcoming days to plan the course of action.

@theseer
Copy link
Member

theseer commented Oct 14, 2023

I very much like the idea of growing phar-io/gnupg to become a more complete solution.

@heiglandreas
Copy link
Collaborator

I wouldn't leave anything to the user!

We are currently using phario/gpg. And I'd only refactor that lib to handle the stuff differently and especially to handle cases where neither the gpg extension nor the gpg binary are present

@theseer
Copy link
Member

theseer commented Oct 14, 2023

Do you plan to plan to have a full userland implementation? 😮

@heiglandreas
Copy link
Collaborator

NO!

More like not verifying at all but telling the user that they are on a system without either and that that is insecure and they are on their own from here!

@theseer
Copy link
Member

theseer commented Oct 30, 2023

Okay, given the meeting in Munich didn't work, let's try to schedule a video call :-)

I'll send a link later today to find time slots.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants