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

Add support for Microsoft POP3 XOAUTH2 #200

Merged
merged 25 commits into from
Nov 18, 2022
Merged

Add support for Microsoft POP3 XOAUTH2 #200

merged 25 commits into from
Nov 18, 2022

Conversation

griesi007
Copy link

@griesi007 griesi007 commented Jul 29, 2022

Signed-off-by: griesi007 noreply@12live.de

Add support for Microsoft POP3 via modern authentication in addition to rejected pull request #199 with required integration tests

$storage = new \Laminas\Mail\Storage\Pop3(
	Laminas\Mail\Protocol\Pop3\Xoauth2\MicrosoftFactory::getInstance(
		'Outlook.office365.com',
		'mail@example.com',
		'accessToken'
	)
);

Signed-off-by: griesi007 <noreply@12live.de>
@Ocramius Ocramius added this to the 2.17.0 milestone Jul 29, 2022
@Ocramius Ocramius requested a review from Slamdunk July 29, 2022 07:52
Christian Ebert added 9 commits July 29, 2022 10:04
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
@Ocramius Ocramius removed this from the 2.17.0 milestone Aug 6, 2022
Copy link
Contributor

@Slamdunk Slamdunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @griesi007 for the improvements over the first PR.
Check the few changes I pinpointed and let's see how the shape looks.
Please rebase the PR as the first step.

src/Protocol/Pop3/Xoauth2/Microsoft.php Outdated Show resolved Hide resolved
src/Protocol/Pop3/Xoauth2/MicrosoftFactory.php Outdated Show resolved Hide resolved
src/Protocol/Xoauth2/Xoauth2.php Outdated Show resolved Hide resolved
src/Protocol/Pop3/Xoauth2/Microsoft.php Outdated Show resolved Hide resolved
test/Protocol/Pop3/Xoauth2/MicrosoftTest.php Outdated Show resolved Hide resolved
test/Protocol/Pop3/Xoauth2/MicrosoftTest.php Show resolved Hide resolved
Christian Ebert added 4 commits August 10, 2022 19:20
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Christian Ebert added 9 commits August 10, 2022 20:05
Signed-off-by: griesi007 <noreply@12live.de>
# Conflicts:
#	src/Protocol/Pop3.php
#	src/Protocol/Pop3/Xoauth2/Microsoft.php
#	src/Protocol/Xoauth2/Xoauth2.php
#	test/Protocol/Pop3/ResponseTest.php
#	test/Protocol/Pop3/Xoauth2/MicrosoftTest.php
#	test/Protocol/Xoauth2/Xoauth2Test.php
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
Christian Ebert added 2 commits August 11, 2022 08:58
Signed-off-by: griesi007 <noreply@12live.de>
Signed-off-by: griesi007 <noreply@12live.de>
@griesi007
Copy link
Author

@Slamdunk : what´s the current status on this pull request? All requested changes habe been applied some time ago. Is it still considered to be accepted and merged?

@Slamdunk
Copy link
Contributor

I'm sorry for the late reply, indeed I forgot this.

I don't know how to handle this properly though: I am not able to check that this PR works, I don't have an Office365 account.

@Ocramius @weierophinney beside the formal correctness of the code and adherence to laminas standards, is trusting the contributor enough to merge a PR with a new feature, when I cannot test it and its test cannot be automated?

@Ocramius
Copy link
Member

@Slamdunk if you are happy with just the unit test, then let's move it forward.

@Slamdunk Slamdunk added this to the 2.20.0 milestone Nov 18, 2022
@Slamdunk Slamdunk self-assigned this Nov 18, 2022
@Slamdunk Slamdunk merged commit ad6889d into laminas:2.17.x Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants