Skip to content

Add support for Microsoft POP3 XOAUTH2#200

Merged
Slamdunk merged 25 commits intolaminas:2.17.xfrom
griesi007:2.17.x
Nov 18, 2022
Merged

Add support for Microsoft POP3 XOAUTH2#200
Slamdunk merged 25 commits intolaminas:2.17.xfrom
griesi007:2.17.x

Conversation

@griesi007
Copy link
Copy Markdown

@griesi007 griesi007 commented Jul 29, 2022 •
edited by Slamdunk
Loading

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'
	)
);

Add support for Microsoft POP3 XOAUTH2
53cf29d 
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
Fixed QA issues
28d0b8e
Fixed QA issues
4aa3901 
Signed-off-by: griesi007 <noreply@12live.de>
Merge remote-tracking branch 'origin/2.17.x' into 2.17.x
dc25746
Fixed QA issues
6f9df39 
Signed-off-by: griesi007 <noreply@12live.de>
Fixed QA issues
194e70c 
Signed-off-by: griesi007 <noreply@12live.de>
Fixed psalm @internal issue
e3c6b7a 
Signed-off-by: griesi007 <noreply@12live.de>
Fixed psalm @internal issue
0a59649 
Signed-off-by: griesi007 <noreply@12live.de>
Fixed psalm @internal issue
607cc02 
Signed-off-by: griesi007 <noreply@12live.de>
Fixed psalm @internal issue
e3d826f 
Signed-off-by: griesi007 <noreply@12live.de>
@Ocramius Ocramius removed this from the 2.17.0 milestone Aug 6, 2022
Slamdunk
Slamdunk requested changes Aug 10, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Comment thread src/Protocol/Pop3/Xoauth2/Microsoft.php Outdated
Comment thread src/Protocol/Pop3/Xoauth2/MicrosoftFactory.php Outdated
Comment thread src/Protocol/Xoauth2/Xoauth2.php Outdated
Comment thread src/Protocol/Pop3/Xoauth2/Microsoft.php Outdated
Comment thread test/Protocol/Pop3/Xoauth2/MicrosoftTest.php Outdated
Comment thread test/Protocol/Pop3/Xoauth2/MicrosoftTest.php
@Slamdunk Slamdunk mentioned this pull request Aug 10, 2022
Open
Christian Ebert added 4 commits August 10, 2022 19:20
Integrated requested review changes
1054f58 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
4a197d3 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
c3c5b5a 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
bc479fd 
Signed-off-by: griesi007 <noreply@12live.de>
@griesi007 griesi007 mentioned this pull request Aug 10, 2022
Closed
Christian Ebert added 9 commits August 10, 2022 20:05
Integrated requested review changes
6358b87 
Signed-off-by: griesi007 <noreply@12live.de>
Merge branch 'XOauth2' into 2.18.x
760dba4 
# 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
Integrated requested review changes
21b83f1 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
4a70ee3 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
e60503c 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
6709360 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
960289c 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
2a8896a 
Signed-off-by: griesi007 <noreply@12live.de>
Integrated requested review changes
6728705 
Signed-off-by: griesi007 <noreply@12live.de>
Christian Ebert added 2 commits August 11, 2022 08:58
Added @Final annotation
7ed638b 
Signed-off-by: griesi007 <noreply@12live.de>
Added @Final annotation
0664ebb 
Signed-off-by: griesi007 <noreply@12live.de>
@griesi007
Copy link
Copy Markdown
Author

griesi007 commented Nov 17, 2022

@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
Copy Markdown
Contributor

Slamdunk commented Nov 17, 2022

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
Copy Markdown
Member

Ocramius commented Nov 17, 2022

@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
This was referenced Nov 18, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Slamdunk Slamdunk Slamdunk requested changes

Assignees

@Slamdunk Slamdunk

Labels

Enhancement

Projects

None yet

Milestone

2.20.0

Development

Successfully merging this pull request may close these issues.

3 participants

@griesi007 @Slamdunk @Ocramius