osTicket / osTicket-plugins

Core plugins for osTicket (v1.8+)
GNU General Public License v2.0
149 stars 162 forks source link

MS OAuth2: Don't force consent prompt #241

Closed teward closed 1 year ago

teward commented 2 years ago

Microsoft OAuth2 endpoints are smart enough that we don't need to tell it to issue a consent prompt.

In a number of cases (I tested this with two separate differently configured differently-secured tenants on MS365 today), defining 'prompt' => 'consent' as one of the url options will force admin consent (this is translated to admin_consent on the MS365 backend - figured out after a 3 hour power session with MS365 admins and support who discovered this).

When you send the OAuth request to MS365, the system is smart enough to determine if you need consent or not, and you do NOT need to tell MS365 that a consent window is required. That may be a requisite for other providers, but not MS365 based on the testing I've done, and what MS365 support said on the 3 hour power-session support call.

wrac4242 commented 2 years ago

Can confirm that this is an issue, and MS support (Support Engineer) says this is the cause. Without this, it requires Application Administrator role from Azure AD, however this should not be required from a security standpoint

JediKev commented 2 years ago

@wrac4242

We are going to make this configurable instead of blatantly disabling it.

Cheers.

wrac4242 commented 2 years ago

What's the ETA on that as this is causing major issues? Also, as stated above it is not required in any form, and MS will automatically imply it. As per a ticket I opened with MS earlier, it is not required and with it present it causes issues

JediKev commented 2 years ago

@wrac4242

No idea atm, we don’t have set release dates, but hopefully it will be included in the next release. In the meantime you can download the raw plugin files from GitHub and make the change manually until we make an official patch.

Cheers.

teward commented 2 years ago

cc @wrac4242 because @JediKev pinged the wrong person. If there is no timeline for making this configurable, then the 'fast' way is to take this as a patch and do a "hotfix" release for MS365 - as is evident here, numerous people and companies are having this issue, making "manually patching this" a poor response.

wrac4242 commented 2 years ago

Is there a reason why that line should be made into an option? As having it there and not having it there has 0 difference

teward commented 2 years ago

Is there a reason why that line should be made into an option? As having it there and not having it there has 0 difference

Note that I concur here, this shouldnt be an "option" and instead you should rely on Microsoft to determine if the prompt is needed or not.

protich commented 2 years ago

Is there a reason why that line should be made into an option? As having it there and not having it there has 0 difference

@wrac4242 We added the forced prompt initially because the Refresh Token can expire when OAuth2 App is in Test Mode which in turn makes Access Token fail to renew. Adding prompt forces a new Refresh Token to be obtained on reauthorization otherwise new Refresh Token is not returned. This is especially the case with Google for example and perhaps not Microsoft.

If there is no timeline for making this configurable, then the 'fast' way is to take this as a patch and do a "hotfix" release for MS365 - as is evident here, numerous people and companies are having this issue, making "manually patching this" a poor response.

@teward - I hate to break your bubble here - but we don't work for anyone. The inconvinience is only on the setup stage and beside that we're planning on a maintenance release next week. Perhaps you'll consider that a poor response as well... but adding OAuth support was a major undertaking with freeloaders simply sitting on the sideline only to make demands post release. We had multple Release Candidates with virtually no one in the community helping to test & give feedback.

wrac4242 commented 2 years ago

I believe this issue only started to be present due to Microsoft's force change over to Oauth2 instead of the previous access methods. This would have been forced after any RC, so there wasn't a reason for people using Microsoft based products to test it during that time.

wrac4242 commented 2 years ago

Hi, does said maintenance release include updates to the plugins? as updates to the core code will not solve this issue I believe. Also with some testing on my side, there is no way at all to get MS to accept any requests without this change present, so adding configurations for this will not change anything and having it set as a default would cause further issues

teward commented 2 years ago

I hate to break your bubble here - but we don't work for anyone.

Never said you did. However, if osTicket wants support with MS OAuth API then they might want to listen to what MS Engineers are telling me is the problem, and for the record I'm doing what MS Engineering asked me to do - follow up with the application developers.

The inconvinience is only on the setup stage and beside that we're planning on a maintenance release next week.

No... it isn't actually. Alluding to my last message, I had a 3 hour power session phone conference with MS Engineers to try and figure out what was breaking the ability for OAuth to work after consent was given org-wide by an administrator, and the MS Engineers themselves indicated that the reason this was not working in any way shape or form was, in fact, the fact we're passing the request with the consent parameters.

During that session, MS Engineers specifically told me on the 3 hour call (AFTER we did 3 hours of debugging and app reconfig on the Azure side only to see that wasn't the issue) that "This is the reason the application consent already set by the administrators is not working is because the request parameter for consent is being sent in the call to MS from the application and as such is forcing a consent prompt". Because of the changes since October 1 forcing OAuth for IMAP, etc. functionality and in MS365's world handle all 'prompt=consent' parameters as another request for admin consent, unless you download the raw plugin, apply this change set, and then manually update the plugin on every affected osTicket instance, the MS OAuth API will not function properly here. This is me simply restating what MS Engineering has said about what the application team needs to do to make their stuff work with MS365.

If your maintenance release does not include an update to the plugin that changes the consent request to be disabled via options, then your application remains unusable with MS365, for the reasons that MS Engineering has indicated. Whether you work for anyone or not (ahem you work for osTicket, I assume, so you work for someone at some level), I would assume that osTicket as an organization and team would want their product to function rather than stagnate and require everyone who is trying to use the OAuth integration with MS365 to roll customized variations of the plugin until some "future point" that osTicket decides to make this configurable as an option.

protich commented 2 years ago

@teward - I wholeheartedly understand the issue and feedback provided and as we indicated the issue will be fixed in the the upcoming maintenance release.

What I reject and rebel aganist is "fix this now or else" type of feedback. You seem frustrated that you spent 3 HOURS on a call with MS to figure out the issue. Do you have an idea how long and effor it took to add OAuth2 Support? If you did then perhaps you wouldn't be salty about your 3 hours.

We thank you for the feeback.