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

Registrant portal API documentation #917

Merged
merged 15 commits into from
Aug 10, 2018
Merged

Registrant portal API documentation #917

merged 15 commits into from
Aug 10, 2018

Conversation

maciej-szlosarczyk
Copy link
Contributor

@maciej-szlosarczyk maciej-szlosarczyk commented Jul 16, 2018

Contains new documentation for Registrant-centered API from #902. Open for discussion. Contact update will be actually implemented in #849.

@vohmar vohmar requested a review from artur-intech July 17, 2018 08:33
@vohmar
Copy link
Contributor

vohmar commented Jul 17, 2018

for registry lock the applied statuses need revision. I suggest applying only clientUpdateProhibited and clientDeleteProhibited. Different question is should we lock down the contacts as well - that would possibly require creating new objects or verifying that the objects are not associated with any other domain before we can do that, so for the first release I would leave it with domain object client side restrictions.

@maciej-szlosarczyk
Copy link
Contributor Author

for registry lock the applied statuses need revision. I suggest applying only clientUpdateProhibited and clientDeleteProhibited.

I took the statuses from https://www.icann.org/resources/pages/epp-status-codes-2014-06-16-en, used all that had a mention of Registry Lock in the table. I'll update it to only include the ones you pointed out.

Different question is should we lock down the contacts as well - that would possibly require creating new objects or verifying that the objects are not associated with any other domain before we can do that, so for the first release I would leave it with domain object client side restrictions.

I wouldn't do that just yet, since as you pointed out there are some things to iron out. A related question is: What is the use case for the fact that contacts should be linkable with multiple (or none) domains? It would make this functionality (and other related ones) a lot easier if each contact object could be attached to exactly one domain.

@vohmar
Copy link
Contributor

vohmar commented Jul 17, 2018

good doc, lets add clientTransferProhibited to the registry lock status list

@@ -21,7 +21,7 @@ Values in brackets represent values that come from the id card certificate.
| first_name | true | String | | Name of the customer (`GN`) |
| last_name | true | String | | Name of the customer (`SN`) |
| country | true | String | 'ee' | Code of the country that issued the id card (`C`) |
| issuing authority | true | String | 'AS Sertifitseerimiskeskus' | |
| issuing_authority | true | String | 'AS Sertifitseerimiskeskus' | |
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just issuer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good thought. However, as I'm looking at it, I think it should be dropped completely as we are not accepting any other CAs than SK.

If needed later for usage with Belgian ID cards, we can restore this parameter as an optional one.

Thoughts?

Copy link
Contributor

@artur-intech artur-intech Jul 18, 2018

Choose a reason for hiding this comment

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

Good idea! +1 for removing it, since it's kinda hardcoded.

@vohmar
Copy link
Contributor

vohmar commented Jul 18, 2018

I am sorry but lets change up the statuses again. The problem is that even though in our case the current set of statuses is correct this would not serve its purpose when registrar side status editing is enabled - statuses with client prefix can be set and removed by registrar in that case. So to make the registry lock work as it supposed to in that case as well we should follow the icann document you referenced above and set serverUpdateProhibited, serverTransferProhibited and serverDeleteProhibited statuses instead.

#### Request
```
POST /repp/v1/auth/token HTTP/1.1
Accept: application/json
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why we need to send JSON request? I think it's much simpler to use it this way

curl https://api.example.com/v1/contacts -d first_name="John -d last_name=Doe

rather than creating JavaScript.

P.S. Applies to the entire API, not only to the line I commented.

Copy link
Contributor Author

@maciej-szlosarczyk maciej-szlosarczyk Jul 23, 2018

Choose a reason for hiding this comment

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

It works both ways, either POST params or a JSON data structure (with properly set Content-Type header). However, as the main target for the API is to programatically integrate a portal, not serve one-off curl requests, I decided to document it with that in mind.

Copy link
Contributor

@artur-intech artur-intech Jul 31, 2018

Choose a reason for hiding this comment

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

Why we need to support both request types? (application/json and application/x-www-form-urlencoded)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to and we do not support both explicitly. It works out-of-the-box unless you specify the following constraint in routes:

namespace :api, constraints: lambda { |req| req.format == :json }

I don't see a reason to introduce it, though.

@vohmar vohmar merged commit 43a7086 into master Aug 10, 2018
@vohmar vohmar deleted the registry-902 branch September 6, 2018 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants