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

REPP: Domain management #1774

Merged
merged 91 commits into from
Mar 29, 2021
Merged

REPP: Domain management #1774

merged 91 commits into from
Mar 29, 2021

Conversation

karlerikounapuu
Copy link
Member

@karlerikounapuu karlerikounapuu commented Dec 9, 2020

Closes #1756

API documentation can be found from https://internetee.github.io/repp-apidoc/

  • Rename admin_domain_contacts_attributes / tech_domain_contacts_attributes
  • Return simple domain dataset for different registrar specific domain (GET domains/:id)
  • Fix auth_code on domain registration
  • Better thorough examples for nameservers and DNSSEC keys
  • Use same variable names where possible (eg registrant and registrant_id)
  • Domain change registrant attribute to type String from Array
  • Renew command sanity check (dates)
  • Get all domain nameservers via query
  • Auth-code, transfer_code, authorization_code - Single naming
  • Fix manipulating with auth_info
  • Remove fax fields

@karlerikounapuu karlerikounapuu force-pushed the repp-domains branch 3 times, most recently from 404e7e1 to 3af5d55 Compare December 29, 2020 13:36
@karlerikounapuu karlerikounapuu force-pushed the repp-domains branch 9 times, most recently from 86e7e0e to a7831af Compare January 7, 2021 10:06
@internetee internetee deleted a comment from OlegPhenomenon Feb 15, 2021
@internetee internetee deleted a comment from OlegPhenomenon Feb 15, 2021
@internetee internetee deleted a comment from OlegPhenomenon Feb 15, 2021
@vohmar
Copy link
Contributor

vohmar commented Mar 4, 2021

  • Get: requesting info on a domain belonging to another registrar returns 2303 - object does not exist. That is not correct. Ideally repp should return the same set of data available in public whois, but to simplify things at this point the response could also be 2305 "Object association prohibits operation" and refer to whois for more information.
    • Doc: under hhtp request description the url has colon in it (GET /repp/v1/domains/:domain_name) - same goes for POST domain update and renew and all the other requests that have domain name in url
  • Post:
    • error in doc (very nice doc by the way - thank you!) - payload parameters description has parameters admin_domain_contact_attributes but the shell example has admin_domain_contacts_attributes (same goes for tech contact attributes)
    • how does admin_contacts_attributes and tech_contacts_attributes or even admin_contacts and tech_contacts. The domain context should be clear from the request and as expected are only contact object ids - no other attributes. Or is there some technical reason for having long variable names like admin_domains_contacts_attributes?
    • the doc would also be a bit more helpful with more complete examples with nameservers and subarrays and dnskeys (I can provide an example or two).
    • setting auth code does not seem to work on domain create, tried with auth_info like in change domain description and auhtorization_code like in domain_info (GET) response.
    • Why is post used throughout with domain operations? in case of contacts the post was used for create and put for update - why is domain different (post vs put)?
    • would it be possible to use same variable names for the same objects in all requests ie in domain create there is "registrant_id": and in domain change "registrant":. Same question is actually about variable names in requests and responses ie the auth_info from previous point.
    • why is the registrant attribute array in domain change - each domain can have only one registrant at a time - so simple string type should work fine here (always replacing one object with another.
    • could not get the domain update to work ie post https://st-repp.infra.tld.ee/repp/v1/domains/repptest5.ee returns 404 not found while get https://st-repp.infra.tld.ee/repp/v1/domains/repptest5.ee works fine
    • contact operations and domain info, create and registrant change requests all start with contact or domain attribute but nameserver, dnskeys and especially renew and transfer do not - looks a bit confusing as all these operations are in this case in domain context. Maybe it would be better to add domain level to those operations as well or on the other hand use create and update names for domain create and domain update operations? But then again url already has the domain/contact and also operation call in it so not entirely sure what would be best here - maybe loose the top level of the query structure if possible? I do understand the need in case of dnssec where there is theoretically possible to offer choice between dnskey and ds approaches. In any case the logic should follow the same lines throughout the implementation.
    • renew - we should add the same kind of sanity check here as in case of epp and require correct current expiration date in the request to make sure its not accidental double renew
  • Nameservers
    • doc - again adding a bit more complex example with two nameservers bein added together with multiple ips per ns might be helpful
  • Dnssec
    • DNSSEC has cool get function for listing assigned dnskey/ds records why don't nameservers have that?
    • add new dnssec record has element public_key for the dnskey value, in epp rfc this element is called pubKey or digest (in case ds value is sent/accepted/supported). Any reason by public_key is better name for the element here - best practice for json naming?
  • statuses
    • clienthold - is it better to use the approach where status is specified in url instead of leaving the url at statuses and then manage what statuses to add or remove in the query. I know we currently only support removing clientHold status, but in bigger picture would it be much more difficult to use approach like in case of nameservers and dnssec?
  • transfer
    • Auth-code, transfer_code, authorization_code - we should try to stick with a single naming
    • why use auth-code as header parameter vs attribute in body in transfer info request (like put transfer request)?
  • Other
    • would it be possible to add nameserver, dnssec and status manipulation also in the domain change request?

Sidenote - few contact endpoint issues:

  • setting and updating auth_info does not work - should it?
  • lets loose fax from REPP

@vohmar vohmar assigned karlerikounapuu and unassigned vohmar Mar 5, 2021
@karlerikounapuu
Copy link
Member Author

  • setting auth code does not seem to work on domain create, tried with auth_info like in change domain description and auhtorization_code like in domain_info (GET) response.

All references to EPP transfer_code auth_info / transfer_info / authorization_code is not referenced as transfer_code. Reserved password for domain creation is names as reserved_pw

  • why is the registrant attribute array in domain change - each domain can have only one registrant at a time - so simple string type should work fine here (always replacing one object with another.

Registrant is an object in domain change because registrar needs to have an ability to submit verified flag

  • Why is post used throughout with domain operations? in case of contacts the post was used for create and put for update - why is domain different (post vs put)?

Mistake in doc, fixed. PUT is meant for object update, while POST is meant for creating an object, in most cases.

  • Any reason by public_key is better name for the element here - best practice for json naming?

Correct. I'm trying to maintain the same (snake_case) key naming approach in the whole REPP protocol as it's default in Rails.

  • would it be possible to add nameserver, dnssec and status manipulation also in the domain change request?

It's possible, sure. I omitted it from the domainChange intentionally to keep down the size of payloads and make the design (and endpoints) as semantic as possible.

@vohmar vohmar merged commit d073656 into master Mar 29, 2021
@vohmar vohmar deleted the repp-domains branch June 8, 2021 13:36
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.

REPP: Domain management
4 participants