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

GDPR - Registrant: Enable user to update contact data of their domain contacts #849

Closed
3 tasks done
vohmar opened this issue May 22, 2018 · 36 comments
Closed
3 tasks done
Assignees

Comments

@vohmar
Copy link
Contributor

vohmar commented May 22, 2018

The goal is to enable users to update their own domain contact data without the need to turn to registrar - each person owns their personal data and should have control over it.

To update the data in the registry db the contact update functionality should be implemented in rest-epp api.

Identically to epp contact update request the user can update only contact data - name, email. phone nr and if postall address processing is enabled postal address as well. User cannot change ident data (ident code, ident type and ident country code)

User can update only their own contact data if a domain has different admin and/or tech contact the user has to turn to registrar to update these details.

To notify registrar about change in domain/contact data we should implement change poll on the epp api (rfc draft: https://tools.ietf.org/html/draft-ietf-regext-change-poll-08).


https://github.com/james-f-gould/EPP-Change-Poll

  • Registrant API
  • EPP Poll
  • Registrant area UI
@vohmar vohmar changed the title Registrant: Enable user to update contact data of their domain contacts GDPR - Registrant: Enable user to update contact data of their domain contacts May 22, 2018
@artur-intech artur-intech self-assigned this Jun 4, 2018
@artur-intech
Copy link
Contributor

artur-intech commented Jun 4, 2018

"GDPR - Registrant" in the title means UI in registrant portal?

@artur-intech
Copy link
Contributor

Does EPP API remain unaffected?

@vohmar
Copy link
Contributor Author

vohmar commented Jun 4, 2018

Yes Registrant means portal for regsitrant, GDPR there to make it look more important - refering to the policy this functionality was called after.

EPP API will remain unaffected for time being as the portal should use REPP API to provide this.

@artur-intech
Copy link
Contributor

artur-intech commented Jun 7, 2018

@vohmar A call to PATCH /api/v2/contacts/xxx will return contact itself on success. I believe we should revise fields that are returned.

I suggest:

  • id (UUID, not EPP code; see Decide what is considered ID in REST API #737)
  • object ("contact" in this case)
  • name
  • email
  • phone
  • fax (This feature can be disabled. I guess it's still worth returning it, but with null as a value)
  • ident
    • code
    • type
    • country_code
  • transfer_code
  • org_name
  • address (can be disabled)
    • city
    • street
    • zip
    • country_code
    • state

See also https://github.com/internetee/registry/blob/master/doc/repp/v1/contact.md#response

@maciej-szlosarczyk FYI

@artur-intech
Copy link
Contributor

artur-intech commented Jun 7, 2018

User can update only their own contact data if a domain has different admin and/or tech contact the user has to turn to registrar to update these details.

@vohmar Could you elaborate? How is it connected to a domain? I probably misunderstood the point.

Please describe the workflow from Registrant Portal user's perspective.

@artur-intech
Copy link
Contributor

artur-intech commented Jun 8, 2018

@vohmar

As I see it, it should look like this (correct me if I am wrong)

As a Registrant Portal user, who has Estonian ID-card I:

  • Open Registrant Portal
  • Go to your profile page (similar to one in Registrar Portal)
  • Edit my details (say, email)
  • Submit the form
  • Expect an associated contact (mapping is done by comparing ID code part from users.registrant_ident with contacts.ident) to be updated with the new email

As I understand, currently there is no reuse, so there will be multiple contacts associated with my ID code, so this complicates a process a bit. Perhaps it's worth to eliminate duplicate contacts?

@artur-intech
Copy link
Contributor

artur-intech commented Jun 9, 2018

To notify registrar about change in domain/contact data we should implement change poll on the epp api (rfc draft: https://tools.ietf.org/html/draft-ietf-regext-change-poll-08).

Why it's needed to notify registrars? Is it safe to rely on a draft version? Perhaps we could implement same feature in REST API?

@vohmar
Copy link
Contributor Author

vohmar commented Jun 11, 2018

Returned fields are open for discussion we have a choice to follow EPP response or expand by looking at what would be beneficial.

Minimum set (with ident data):

  • code - this is contact object code
  • id - this can be the UUID instead of db unique id
  • statuses - array of active statuses
  • name
  • ident
  • ident_type
  • ident_country_code
  • address (should be possible to disable according to Address processing setting)
    • street
    • city
    • state
    • zip
    • country_code
  • phone
  • fax (should be possible to disable, we have no point in returning null as we do not collect this data)
  • email
  • registrar_id - current registrar code
  • creator_str - code of the registrar that created the object
  • created_at
  • updator_str - id of the registrar that did the last update - we will create new registrar for this ie with code Registrant for all updates made in the portal for registrants
  • updated_at
  • auth_info

extra:

  • status notes

legacy_id can be dropped

Workflow for update contact information

  1. User logs in to the portal for registrants
  2. selects domain form the list of domains
  3. in the domain details view click on the contact he/she wants to update the contact details of
  4. user is taken to contact details view where he/she can update phone number and email address of that object, and postal address if address processing is enabled. User also sees what domains and roles are affected by that change (similar to the contact details view in the registrars' portal)
  5. user clicks save - popup is returned asking if user is sure and aware that listed domains and roles are affected by that change. If OK/yes is the choice the change is made, registrars are notified and whois is updated.

Notifying registrar is necessary because the object it self still is owned by the registrar so they might be interested if their client updates their contact data. We do not have better alternative to use than the draft version of the change poll rfc - so we will accept the risk that the solution may change before becoming a standard.

@artur-intech
Copy link
Contributor

artur-intech commented Jun 12, 2018

According to this view http://prntscr.com/jtvnpl (https://projects.invisionapp.com/share/T7HB9WYZSYV#/screens/287337990 not sure if this link points to the correct view; see previous one to be 100% sure we're on the same page) there is a separate page for managing contacts, but you mentioned "domain details" view instead. Does it mean it should be possible to edit contacts from both places?

Please review this question again: #849 (comment)

The main point of that question: is it really needed to enable managing all those contacts http://prntscr.com/jtvp0y, given they all will probably contain same data (I login with ID-card, so I will only see contacts connected to my ID code, it means they all will share same address, same email etc)?Correct me if I am wrong.

@artur-intech
Copy link
Contributor

artur-intech commented Jun 12, 2018

In general, I would start with a blank state (instead of EPP) and add additional fields as needed.

Say it is not clear, why do we need these fields:

  • EPP code (how EPP code could be handy in REST API?)
  • registrars.code (how this field can be useful to a domain owner?) Perhaps the name of a registrar might work better?
  • created_at, updated_at currently these fields are updated on every modification of a database row. There might be some cases where a change is not initiated by a customer/user/registrar, so it may potentially lead to some confusion for them

@artur-intech
Copy link
Contributor

artur-intech commented Jun 12, 2018

Workflow for disclosing private data in whois using the portal for registrants

I would like to see the workflow of editing contact details. I guess the one you mentioned is not directly connected to the latter?

@vohmar
Copy link
Contributor Author

vohmar commented Jun 12, 2018

In general, I would start with a blank state (instead of EPP) and add additional fields as needed.

we cannot do this if we would like the registrars to be able to use this api as well.

  • contact object code is necessary as all the references to domain objects for example are done using this value
  • i do agree that any kind of code for registrars or contacts is not useful for registrants. So the option is to add registrar name as well to the results (creator, current and last one to update the object)
  • created and updated_at is papertrail issue, but it should be possible to get the correct value for create date and last update regardless first from the initial create object record and the latter from the latest record.

Updated the workflow in the comment: #849 (comment)

@artur-intech
Copy link
Contributor

artur-intech commented Jun 14, 2018

user clicks save - popup is returned asking if user is sure

I don't see a point of such confirmation dialog. 99% of users will click "OK" without even reading it.

@artur-intech
Copy link
Contributor

artur-intech commented Jun 15, 2018

created and updated_at is papertrail issue

These columns have nothing to do with PaperTrail. How an end user could benefit from them?

updator_str - id of the registrar that did the last update - we will create new registrar for this ie with code Registrant for all updates made in the portal for registrants

Can you clarify this?

@artur-intech
Copy link
Contributor

artur-intech commented Jun 15, 2018

creator_str - code of the registrar that created the object

This field contains user, not registrar. Did you mean the former? Why is it needed?
Same applies to updator_str.

These are connected to PaperTrail. If we use them, it will be more difficult and time consuming to extract it later.

@vohmar
Copy link
Contributor Author

vohmar commented Jun 21, 2018

I don't see a point of such confirmation dialog. 99% of users will click "OK" without even reading it.

It is important to let user know that they will reveal their personal data on multiple domains if that is true. That is their decision to read the notice or not.

How an end user could benefit from them?

Registrars have a right to know that their domain data has been edited, when and by whom

Can you clarify this?

If registrant changes its contacts using portal for registrants we will put registrant down as the updator - the id of the registrar. So registrant Jaan updates their contact details regarding the domain jaan.ee using the portal for registrants. after the update domain and contact details will show 2018.06.21 10:21 as the last update time and "Registrant" as the updator. But If you are saying that it is api user that is revealed there then that is not ideal and you can hide that until we have a replacement for Papertrail.

@maciej-szlosarczyk
Copy link
Contributor

maciej-szlosarczyk commented Jul 16, 2018

The main point of that question: is it really needed to enable managing all those contacts http://prntscr.com/jtvp0y, given they all will probably contain same data (I login with ID-card, so I will only see contacts connected to my ID code, it means they all will share same address, same email etc)?Correct me if I am wrong.

These contacts need to be updated separate, we cannot unify them. A very typical case:

  1. John Smith owns a domain that has set contact data to his private email, phone and home address.
  2. John Smith is the member of the board of Company Z, where he uses company email, phone and address as the contact data.
  3. John Smith is also a member of the board of the Estonian Rugby Association, where he uses his private phone, but an email address and postal address provided by the Association.

@artur-intech
Copy link
Contributor

artur-intech commented Aug 2, 2018

and if postall address processing is enabled postal address as well

@vohmar I guess the same applies to fax?

@artur-intech
Copy link
Contributor

artur-intech commented Aug 19, 2018

address (should be possible to disable according to Address processing setting)
fax (should be possible to disable, we have no point in returning null as we do not collect this data)

As discussed, those will be returned regardless of respective settings.

@ratM1n
Copy link

ratM1n commented Sep 24, 2018

if i update my contacts from registrant portal, i will get error:
Sep 24 10:19:38 staging REGISTRANT[1537]: [st-rant.infra] [d31ecdec-dd2f-44bc-b1ba-3d35e8c79adb] [XXX.XXX.XXX.XXX] #012NoMethodError (undefined method request_uri' for #URI::Generic:0x00007fdb8c597878):#12 app/controllers/registrant/contacts_controller.rb:121:in new'#012 app/controllers/registrant/contacts_controller.rb:121:in update_contact_via_api'#12 app/controllers/registrant/contacts_controller.rb:20:in update'

@artur-intech
Copy link
Contributor

artur-intech commented Sep 26, 2018

I am sorry, I forgot to mention that application.yml got updated.

registrant_api_url: 'https://api.registry.example'

#850 is to eliminate such issues (@vohmar FYI).

It's worth mentioning that currently HTTPS in that URL is not supported. I guess it's needed to implement it still. For this we probably need new config entries to specify key paths for Registrant API, since cert_path and key_path are used to access Registrar API. @teadur Please confirm.

In the meanwhile, you could test it using plain HTTP.

@ratM1n
Copy link

ratM1n commented Sep 27, 2018

this error:
Sep 27 11:33:29 staging REGISTRANT[827]: [st-rant.infra] [e790d2d9-9fa2-49b3-873b-a4916f0415b3] [192.168.2.45] #012Net::HTTPBadResponse (wrong status line: "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML 2.0//EN\">"):#012 app/controllers/registrant/contacts_controller.rb:84:in block in access_token'#12 app/controllers/registrant/contacts_controller.rb:83:in access_token'#012 app/controllers/registrant/contacts_controller.rb:122:in update_contact_via_api'#12 app/controllers/registrant/contacts_controller.rb:20:in update'
points to certificates problem? (i had to use https because registrant portal doesnt listen on http)

@artur-intech
Copy link
Contributor

artur-intech commented Sep 27, 2018

Most probably. It does not work with HTTPS yet anyway.

@teadur
Copy link
Contributor

teadur commented Sep 28, 2018

I am sorry, I forgot to mention that application.yml got updated.

registry/config/application-example.yml

Line 100 in 029bd10
registrant_api_url: 'https://api.registry.example'

#850 is to eliminate such issues (@vohmar FYI).

It's worth mentioning that currently HTTPS in that URL is not supported. I guess it's needed to implement it still. For this we probably need new config entries to specify key paths for Registrant API, since cert_path and key_path are used to access Registrar API. @teadur Please confirm.

In the meanwhile, you could test it using plain HTTP.

Yes cert_path and key_path are used in registrar application to access epp.
So new variables need to be introduced for that functionality.
New variables should reference the usage where the certs are used.

@artur-intech
Copy link
Contributor

Done.

registrant_api_client_cert_path:

@ratM1n
Copy link

ratM1n commented Sep 28, 2018

i understand the need for certificates if some other application will access (remotely) registrnat portal api, but why this contact update fails if its IN registrant portal? and how can certificates improve that?

@artur-intech
Copy link
Contributor

artur-intech commented Sep 28, 2018

I am not sure whether the error above is just because of previously unsupported HTTPS, but in general, Registrant Portal uses Registrant API exactly in the same way as any other client. It is in fact a client. It is designed to be used remotely.

Let me know if the error persists even after latest commit, I will investigate further.

@artur-intech
Copy link
Contributor

artur-intech commented Sep 28, 2018

I hope we will soon extract Registrant Portal from registry app. I find it quite difficult to design proper interactions between a client and API where they both are in the same monolith app.

@vohmar
Copy link
Contributor Author

vohmar commented Oct 1, 2018

@artur-beljajev problem remains. Please contact @ratM1n to discuss and explain the current solution. The necessity of separate certificates is still unclear.

@vohmar vohmar assigned artur-intech and unassigned vohmar Oct 1, 2018
artur-intech pushed a commit that referenced this issue Oct 7, 2018
@vohmar
Copy link
Contributor Author

vohmar commented Oct 8, 2018

attempts to edit contact data still fail

Oct 8 14:32:36 staging REGISTRANT[10386]: [st-rant.infra] [b779a848-5e67-4baf-9489-00431d2cf300] [192.168.2.45] #012JSON::ParserError (751: unexpected token at '#12#012<title>403 Forbidden</title>#12#012

Forbidden

#12

You don't have permission to access /v1/registrant/auth/eid#012on this server.

#12
#12Apache/2.2.22 (Debian) Server at st-rant.infra.tld.ee Port 443#12#012'):#12 app/controllers/registrant/contacts_controller.rb:87:inaccess_token'#12 app/controllers/registrant/contacts_controller.rb:122:in update_contact_via_api'#12 app/controllers/registrant/contacts_controller.rb:20:inupdate'`

artur-intech pushed a commit that referenced this issue Oct 8, 2018
artur-intech pushed a commit that referenced this issue Oct 8, 2018
@vohmar
Copy link
Contributor Author

vohmar commented Oct 17, 2018

now receiving timeout on update attempt

Oct 17 15:23:22 staging REGISTRANT[23923]: [st-rant.infra] [d8a60496-3468-45bf-b8d0-e75797a378fd] [192.168.6.34] Completed 500 Internal Server Error in 60211ms (ActiveRecord: 122.4ms)
Oct 17 15:23:22 staging REGISTRANT[23923]: [st-rant.infra] [d8a60496-3468-45bf-b8d0-e75797a378fd] [192.168.6.34] #012Net::ReadTimeout (Net::ReadTimeout):#012  app/controllers/registrant/contacts_controller.rb:84:in `block in access_token'#012  app/controllers/registrant/contacts_controller.rb:83:in `access_token'#012  app/controllers/registrant/contacts_controller.rb:122:in `update_contact_via_api'#012  app/controllers/registrant/contacts_controller.rb:20:in `update'

@artur-intech
Copy link
Contributor

@ratM1n What registrant_api_base_url setting is set to? Is it accessible by Registrant Portal instance?

@ratM1n
Copy link

ratM1n commented Oct 18, 2018

@artur-beljajev registrant_api_base_url: 'https://st-rant.infra.tld.ee' same as we access from browsers and yes, it is accessable and IP is in registrant_api_auth_allowed_ips list. Also, I can access registrant api from my coputer with same URL and it works.

@vohmar vohmar reopened this Oct 19, 2018
@vohmar
Copy link
Contributor Author

vohmar commented Oct 19, 2018

@artur-intech
Copy link
Contributor

artur-intech commented Oct 19, 2018

See #849 (comment)

What exactly should be updated?

@vohmar
Copy link
Contributor Author

vohmar commented Oct 23, 2018

doc updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants