RESOStandards / transport

RESO Transport Workgroup - Specifications and Change Proposals
https://transport.reso.org
Other
18 stars 15 forks source link

RCP-041: oAuth2 Authorization Code support #25

Closed grispin closed 2 years ago

grispin commented 2 years ago

A proposal to add oAuth2 Authorization Code workflow to supported authentication methods

grispin commented 2 years ago

While in an ideal world it would be nice to require only long-term, machine-friendly methods be used, most of the large scale commercial oAuth providers used by many data vendors do not provide end users a method to generate these machine-friendly authentication credential methods without data vendor endorsement/blessing of the integration or limited lifecycle times. Client Credentials are often compared to business-to-business integrations and bearer tokens are the results of a previous authentication challenge done elsewhere.

If we include this only as an endorsement it will largely unimplemented which will result in its effectiveness being near zero as it cannot be counted on by a would-be integrator that does not have a direct integration with the data vendor.

Here is an example use case: a photographer that provides a website to view photos for a new listing where the agent can log into and approve the photographer to upload the photographer's photos to a listing on behalf of the agent/broker.

darnjo commented 2 years ago

@grispin providers MAY use OAuth2 auth code flow, even currently, but MUST support one of a) bearer tokens or b) client credentials for certification.

I'm not sure I see this changing, as we intentionally removed it as a third option from Web API Core not too long ago with support from the community, especially data consumers who couldn't do their normal business because some providers didn't offer any other option.

Servers MUST offer the ability for consumers to replicate data in a machine friendly way to be certified for Web API Core, Data Dictionary, and Payloads specifications.

Providers large and small both support either or (currently 80% bearer tokens and 20% client credentials, with 20 providers and hundreds of markets), so this doesn't seem like it's been an impediment in terms of implementations, with third-party auth layers readily providing support for either. Bearer tokens and Client Credentials are both common standards implemented in things like Postman as well.

I'll defer to @psftc to address this further and explain the reasoning.

If we want to create a new specification so that we can certify providers who also support OAuth2 auth code flow, that would be the recommendation at this point.

It also means some changes to the certification tools, which will no longer be able to run unattended in a Docker container with a config, but rather require an interactive prompt so that someone running the tests could provide credentials, and the token would have to be extracted and persist for long enough to consume up to the full data set from the server so they can run unattended without any further prompts. Much like data consumer processes, testing is designed to be automated.

For the case you mentioned where a photographer wanted to upload an image and could provide a login, the optional endorsement would work fine. We just don't want that to be one of the default auth options for MLS data providers for certification.

If an MLS provider had already passed testing with Bearer Tokens or Client Credentials, then the additional OAuth2 auth code flow might be an available for them, but not until we had successfully been able to replicate large amounts of data in a machine friendly way.

For non-MLS providers, we could potentially have a Web API Core endorsement that didn't require one of these options, but we'd have to make sure to define the policy well. The common case at the moment is MLS vendors who provide feeds to data consumers, so we need to ensure that mechanism always works correctly at a minimum.

darnjo commented 2 years ago

@grispin @psftc - thinking about it a bit further, we had discussed a "Lightweight API" feature as part of the Core 2.1.0 specification (#22). Perhaps we could branch that off into its own specification that allowed providers to pass without demonstrating support for all required Core data types, and additional auth methods like OAuth2 auth code flow?

Then we could say that all MLS providers MUST pass Core, others MAY pass Core, and either could obtain the "Lightweight API" endorsement, once any other requirements had been met?

In terms of cert, the applicant would have to log in through a form, obtain a token, and paste it into a form so it could be used for the testing configuration, and it would have to last the duration of the tests, which could be hours.

One way to automate this would actually be to standardize the ID of the form element that people might use for auth, but that probably goes against the spirit of the protocol.

TL;DR I think there's a way to properly support OAuth2 auth code flow, while requiring a machine friendly mechanism for Core for providers we want to guarantee this functionality for. There are definitely other use cases to support, however.

grispin commented 2 years ago

The problem is that auth code path is optional today and current implementations counts show that support for it is limited. This is likely be not part of the spec and an endorsement carries little more weight than it being optional. For auth code to be a value to clients, it has to be more universal so data can be accessed consistently. As long as it's only an endorsement, it carries doesn't impact certification and will have limited adoption. Implementers will not plan against a model will a small adoption base.

Auth Code testing is tricky which is why I left that part relatively undocumented in the RCP proposal. I'm not sure the best way to test an interactive component. I'm not wanting to take away the machine-friendly approaches or their requirement, they have their place. I want to extend the standard to also include auth code as required so that clients can do more with the data (as the data vendors access license permits). I would want to see both approaches supported.

Ideally, an auth code supported endpoint could be provided when the certification individual is populating the rest of the validating servers credentials. The user populating the certification tool would go through the auth code workflow at that time to generate a token at that time along with the refresh token to keep it alive long enough to continue testing. Revalidation becomes a little harder as larger oAuth providers do not keep auth code tokens for more than a few months. Okta lifetimes for example Other SPs are equally as restrictive due for good security reasons.

darnjo commented 2 years ago

I'm not debating the potential value for some use cases, what I'm saying is that perhaps it doesn't belong in the Core specification at this time?

We voted to remove auth code flow from the Web API Core specification in the last version. I don't see why we'd add it back into the next version?

We also have other priorities at the moment, so I'm not necessarily a fan of adding things that add complexity to the spec at the last moment when we have settled on other clearly defined goals for 2.1.0:

Adding support for auth code flow would delay the work we have to do for Web API Core, DD 2.0, and Payloads 2.0 and there's only been one request for it at this point since we removed it from the prior specs. We should establish the business case in the community before it's added to the Web API Core spec, as has been done in the other cases.

You can support auth flow currently in your applications with your Web API implementation, we just don't certify it at the current time.

In terms of how certification might work, I don't see how it would be implemented other than to offer an interactive prompt. That's how auth code flow is intended to function. See this documentation for further information.

grispin commented 2 years ago

We voted to remove auth code so that machine-friendly authentication was a MUST in the standard and that was a good thing but in hind sight was overly drastic. The original change should have been to update it from OR to an AND for authentication mechanisms.

With out it being part of core, auth code would never get wide spread adoption which makes counting on it for clients a problem. I'm not approaching this problem as a vendor but as a client. I do not have administrative control of the data vendor to extend the implementation.

Most vendors do not write their own oAuth SP and all major oAuth SPs support all auth code. The impact should not be that much for most vendors and we could make it a SHOULD for 2023 and a MUST for 2024 if the group would prefer that. It would show direction atleast.

EnFinlay commented 2 years ago

I'm not in favour of adding Authorization Code support as a MUST because that assumes that all Web API providers are filling a role that requires that functionality, which is not the case. To my knowledge, all Web API providers are in a role that requires a machine-friendly protocol, which matches the MUST in the spec. There might be an argument for requiring Authorization Code support for the Add/Edit endorsement.

grispin commented 2 years ago

When WebApi was originally introduced we ( the RESO community) wanted the transport to be usable for mobile. This was one of the big complaints with RETS, the older protocol was extremely difficult to work with directly from a mobile device due to technology decisions and availability at the time of inception. This made mobile one of WebApi's original goals but the removal of auth code validation makes that goal much more difficult. The lack of it focuses the protocol on application-to-application communication which is important but not the only use-case for WebApi.

Many Mobile use-cases would fall outside of the add/edit endorsement. The example I gave before was only one of many and used for illistrative purposes. The real goal here is to make the data as accessible as it can be to those that license it and lower the barriers to implementation.

darnjo commented 2 years ago

It might be more productive to continue this discussion in the workgroups rather than in this thread.

Just because the Core endorsement doesn't include auth code flow doesn't mean we can't support it in other ways. The question is whether it belongs in Core. The workgroups have already voted to remove it in the past, so my feeling is that it doesn't belong here and especially in the upcoming version with only one person asking for it.

The more appropriate path seems to be making it part of its own optional endorsement, as mentioned previously. The groups had mentioned the possibility of a "live query" endorsement in the past. Something like this would be more appropriate with auth code flow since it's more of an interactive use case.

grispin commented 2 years ago

Do we have a list of proposed endorsements?

darnjo commented 2 years ago

@grispin the current list of proposed endorsements from Web API Core 2.0.0 (formerly known as 1.0.2) and DD 1.7+ is in these PRs and GitHub issues in this repository.

Prior endorsements were either merged with the Data Dictionary 1.7 or Web API Core specifications, or left tabled and are available in the RESO Confluence Member's Transport area under "Legacy Change Proposal Center."


As an aside, in terms of process, RCPs had previously started as posts in Confluence and/or discussions, and then were put on the agenda before the specifications were created.

The process now is to create a ticket first, outlining business cases and requested functionality, which thank you for doing in these cases, and then adding it to the agenda for workgroup discussion.

The Web API Core 2.1.0 ticket is here, for example, and is following a similar process.

Items marked TBD still need discussion to see if they'll make it in this proposal, and the others have already been discussed and approved. Any additional items targeting Web API Core should be added to that proposal, once it starts moving forward, rather than from the Core 2.0.0 branch, since some content will likely change.

The important thing is community participation, so if you feel that making a draft PR is the best way to express the changes you want, that's definitely acceptable. The important thing is to discuss and come to consensus about the business needs where to address them in the specifications prior to their adoption.

Personally, I support the idea of offering other standard forms of auth in the Web API, and appreciate the time spent on the proposal. I'm just not sure if Core is the place to put it. TBD. And, if so, we need to do so in a way that ensures the minimum "Core" requirements we're trying to enforce for our 99% case, MLS Vendors, are being met effectively.

grispin commented 2 years ago

Honestly, with the migration to Github I did not know what the new submission process was. At the RESO conference in Tuscon, I asked and was told to get the RCP written so it could be reviewed and the only documentation that was directly apparent was Submission Process in the readme.md which focuses on the draft PR

darnjo commented 2 years ago

Hi @grispin - your following the submission process is appreciated, so thanks for doing so, and to be clear I'm not opposed to adding this functionality to one of the specifications, just not sure where it would go at this point. My previous comments were more about process than new features. I think the RCP process might still need some fine tuning in this regard.

My questions mostly came up here as a result of the current branch naming strategy: they would suggest we should add this to the next version of Web API Core (2.1.0) since it's based on 2.0.0.

We're pretty close to finalizing those requirements at this point, so we'd need the community to decide whether this should be in the next version or not, or part of Core. There's also a bit of overhead in developing the certification tools, and we don't have implementations yet, so I would think the request wouldn't be targeted to any one spec or version until that point.

What we want to capture is the request and business requirements and have the process stay out of the way until it's necessary so this is where we could improve things more. Perhaps this is best done in an issue or discussion topic, at times, before the spec is written, or perhaps it's iterated in the PR or branch. Something we should discuss further.

We want to make this easy for the community to make new changes, but we also have roadmaps planned that we're trying to follow based on previous work, and want to make sure we can iterate quickly on those requirements in each successive version of the spec.

grispin commented 2 years ago

@darnjo If this needs to wait for 2.2 or 2.3 then so be it, I was never saying this must be in the next release just that it should be considered as we have dropped one of the original use cases for WebAPI. I am hopeful we can make it core so adoption is universal but that is a different topic we will discuss later. As I said before, I was just following the proposal process on the git project which starts with the creation of a branch. IF we want to change the proposal process then lets do so and communicate it in the readme.md so everyone is on the same page.

darnjo commented 2 years ago

Thanks for the clarification, Geoff. It's a good suggestion. If anything, we probably just need to work on our process a bit and separate the requests and business needs from where an item goes. Of course the requested functionality could still target a given endorsement, but it probably shouldn't do so by default.

Perhaps a discussion post first, chat about it in the groups and in the thread, create a ticket once approved, a spec, then create the PR.

We'll discuss process further in the June 2022 Transport meeting.

grispin commented 2 years ago

There are already a ticket that I created at the same time I created the pull requests as I could not find where to get RCP enumeration from. I took that as the place to reserve them based on the other tickets. We can put the discussion there if you prefer.