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

Check if instance is not yet installed #14324

Merged
merged 2 commits into from
Feb 18, 2015
Merged

Check if instance is not yet installed #14324

merged 2 commits into from
Feb 18, 2015

Conversation

LukasReschke
Copy link
Member

Due to a security hardening in 8.1 a missing value of empty trusted domains in the config would provoke an error as this was misused by a lot of users.

This caused a problem where the initial installation happened from another domain than 127.0.0.1 as in this case the domain was considered untrusted as no value was defined. However, this special case should not get intercepted.

To test:

  • Installing ownCloud on 127.0.0.1 works
  • Installing ownCloud on another domain / IP works
  • When setting up ownCloud from 127.0.0.1 and accessing it from the domain above the trusted domain error should be shown if not specified in the config

Fixes #14320

@SergioBertolinSG Please test

Due to a security hardening in 8.1 a missing value of empty trusted domains in the config would provoke an error as this was misused by a lot of users.

This caused a problem where the initial installation happened from another domain than 127.0.0.1 as in this case the domain was considered untrusted as no value was defined. However, this special case should not get intercepted.

To test:
- [ ] Installing ownCloud on 127.0.0.1 works
- [ ] Installing ownCloud on another domain / IP works
- [ ] When setting up ownCloud from 127.0.0.1 and accessing it from the domain above the trusted domain error should be shown if not specified in the config

Fixes #14320
@ghost
Copy link

ghost commented Feb 18, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/9693/
Test PASSed.

@SergioBertolinSG
Copy link
Contributor

While opening the installation page from a domain != localhost, installation page is opened, good. But after that webpage not available appears and you can't enter owncloud.

Config file created after the installation:

<?php
$CONFIG = array (
  'instanceid' => 'oc0hqre4cl0z',
  'passwordsalt' => 'blahblah',
  'secret' => 'blahblah',
  'trusted_domains' => 
  array (
    0 => NULL,
  ),
  'datadirectory' => '/var/www/html/owncloud_fix14320/data',
  'overwrite.cli.url' => 'http:///owncloud_fix14320',
  'dbtype' => 'mysql',
  'version' => '8.0.0.9',
  'dbname' => '22222',
  'dbhost' => 'localhost',
  'dbtableprefix' => 'oc_',
  'dbuser' => 'oc_22222',
  'dbpassword' => 'blahblah',
  'installed' => true,
);

@LukasReschke
Copy link
Member Author

Remark to myself: Always check if it shows more than the installer 🤦

Should be fixed…

@SergioBertolinSG
Copy link
Contributor

Working fine, #14321 is fixed here.

In the third option I faced #14320, the button doesn't add the domain.
(When setting up ownCloud from 127.0.0.1 and accessing it from the domain above the trusted domain error should be shown if not specified in the config)

@LukasReschke
Copy link
Member Author

In the third option I faced #14320, the button doesn't add the domain.
(When setting up ownCloud from 127.0.0.1 and accessing it from the domain above the trusted domain error should be shown if not specified in the config)

How are you accessing it? – The URL to add trusted domains is generated via the first trusted domain.

So if you setup ownCloud under "owncloud.com" and then access it via "mycloud.com" the link will point to "owncloud.com" and the instance has obviously to be accessible for that. In your case I suspect that the URL that is generated is not accessible, can you take a look at the source and tell me where the URL points to?

(Maybe it's pointing to localhost but you cannot reach it from the other machine…?)

@scrutinizer-notifier
Copy link

The inspection completed: 1 updated code elements

@SergioBertolinSG
Copy link
Contributor

@LukasReschke Yes, totally right, that was my problem. Machine wasn't accesible from second domain. Maybe an error message would be interesting in that case?

@ghost
Copy link

ghost commented Feb 18, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/9704/
Test PASSed.

@LukasReschke
Copy link
Member Author

@LukasReschke Yes, totally right, that was my problem. Machine wasn't accesible from second domain. Maybe an error message would be interesting in that case?

Let me see if I can add some JS magic at this state. However a different issue, I created #14332 for this. Not high priority for me right now though.

@LukasReschke
Copy link
Member Author

Another easy one to review.

@th3fallen Can I motivate you to test this and take a look at the changeset? 😄

@th3fallen
Copy link
Contributor

changes seem good 👍

th3fallen added a commit that referenced this pull request Feb 18, 2015
Check if instance is not yet installed
@th3fallen th3fallen merged commit c4fdb9c into master Feb 18, 2015
@th3fallen th3fallen deleted the fix/14320 branch February 18, 2015 14:40
@lock lock bot locked as resolved and limited conversation to collaborators Aug 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Add trusted domains" button doesn't work
4 participants